Skip to content

read(void *pBuf, size_t size) ignores size parameter, causing buffer overflow #3

@walterdobson

Description

@walterdobson

The multi-byte read(void *pBuf, size_t size) function ignores the caller-provided size parameter and instead reads ALL available data into the buffer, causing buffer overflows when available() exceeds the actual buffer size.
Affected Code
File: DFRobot_IICSerial.cpp, lines 143-151
size_t DFRobot_IICSerial::read(void *pBuf, size_t size){ if(pBuf == NULL){ DBG("pBuf ERROR!! : null pointer"); return 0; } uint8_t *_pBuf = (uint8_t *)pBuf; size = available() - (((unsigned int)(SERIAL_RX_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail)) % SERIAL_RX_BUFFER_SIZE); // ⚠️ BUG: Overwrites the size parameter! readFIFO(_pBuf, size); return size; }
Expected Behavior
When calling uart.read(buffer, 64), the function should read at most 64 bytes into the buffer, respecting the caller's buffer size limit.
Actual Behavior
The function ignores the size parameter passed by the caller and recalculates it based on available(). Since available() can return up to 256 (FIFO) + SERIAL_RX_BUFFER_SIZE bytes, this causes the function to write far more data than the buffer can hold.
Impact
This bug causes stack buffer overflows leading to:
Stack smashing protection failures
System crashes/reboots
Unpredictable memory corruption
In our ESP32 embedded application, this was causing consistent crashes approximately 30-60 seconds after boot when sensor data accumulated in the FIFO.
Steps to Reproduce

  1. Create a buffer smaller than potential available data: uint8_t buffer[64];
  2. Wait for sensor to accumulate >64 bytes of data
  3. Call uart.read(buffer, 64);
  4. Observe that more than 64 bytes are written to the 64-byte buffer

Suggested Fix
size_t DFRobot_IICSerial::read(void *pBuf, size_t size){ if(pBuf == NULL){ DBG("pBuf ERROR!! : null pointer"); return 0; } uint8_t *_pBuf = (uint8_t *)pBuf; size_t availableBytes = available() - (((unsigned int)(SERIAL_RX_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail)) % SERIAL_RX_BUFFER_SIZE); size_t toRead = (availableBytes < size) ? availableBytes : size; // Respect caller's buffer size! readFIFO(_pBuf, toRead); return toRead; }

Workaround
Until this is fixed, use the single-byte read() function instead:
// Instead of: uart.read(buffer, size); for (int i = 0; i < size && uart.available() > 0; i++) { buffer[i] = uart.read(); }

I hope this is helpful. If I'm just using it wrong let me know. thanks. Walter

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions