fix: cap dlt_buffer_get copy size to max_size to prevent overflow (#829)

[aki1770-del]

Problem

dlt_buffer_get() in src/shared/dlt_common.c logs a warning when head.size > max_size but still passes head.size to dlt_buffer_read_block(), writing past the end of the caller's buffer — CWE-119 heap/stack overflow depending on the call site.

Reproducer (from issue #829):

// write block 20% larger than DLT_DAEMON_RCVBUFSIZE
unsigned char write_data[DLT_DAEMON_RCVBUFSIZE + 2004] = {0};
static char data[DLT_DAEMON_RCVBUFSIZE] = {0};

dlt_buffer_push(&buf, write_data, sizeof(write_data));
dlt_buffer_get(&buf, (unsigned char *)data, sizeof(data), 0);
// → writes DLT_DAEMON_RCVBUFSIZE + 2004 bytes into a DLT_DAEMON_RCVBUFSIZE buffer

Fix (src/shared/dlt_common.c)

Three changes, one function:

1. Cap the copy: introduce copy_size = min(head.size, max_size) and pass it to dlt_buffer_read_block().

2. Ring buffer consistency: when clipping occurs and delete=1, advance the ring buffer read pointer by the full head.size (not copy_size) so subsequent reads start at the correct position.

3. Return value: return copy_size instead of head.size to correctly report the number of bytes written to the caller's buffer.

+18 / -10 lines, 1 file.

Fixes #829.

[Copilot]

Pull request overview

Fixes a buffer overflow in dlt_buffer_get() by capping the number of bytes copied into the caller-provided buffer when the stored block is larger than max_size, while keeping ring-buffer read pointer advancement consistent when deleting entries.

Changes:

• Introduces copy_size = min(head.size, max_size) and uses it for the data copy to prevent overflow.
• When clipping and delete=1, advances the ring buffer read pointer by the full head.size (not just copy_size).
• Returns copy_size to reflect the actual number of bytes written to the caller buffer.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

max_size is an int, so if a caller accidentally passes a negative value, the current logic will set copy_size = max_size (since max_size is truthy and head.size > max_size), and then cast that negative copy_size to unsigned int for dlt_buffer_read_block(), leading to a very large copy/read and memory corruption. Please treat max_size <= 0 as invalid (or as “no copy”) by changing the checks to max_size > 0, and ensure copy_size cannot become negative before casting to an unsigned size type.

[Copilot]

The wrap logic if ((unsigned int)next_read >= buf->size) next_read -= buf->size; changes a valid pointer value of exactly buf->size into 0. Elsewhere this code treats read == buf->size as in-range (e.g., the pointer range check allows == buf->size), and empty-buffer validation expects write == read when count == 0. If write ends up as buf->size and read gets normalized to 0, the next call will log an error and reset the buffer even though it’s actually empty. Consider matching the existing semantics by only wrapping on > buf->size (not >=), or by reusing the same <= / else pattern used below when advancing read by head.size.

[e-souza5]

I believe that updating the read pointer based on the real copy size is a mistake since it won't point to the next header.