Skip to content

Comments

Fix overflow if history size is integer multiple of entries#13

Open
f-adler wants to merge 2 commits intodimmykar:developfrom
f-adler:develop
Open

Fix overflow if history size is integer multiple of entries#13
f-adler wants to merge 2 commits intodimmykar:developfrom
f-adler:develop

Conversation

@f-adler
Copy link

@f-adler f-adler commented Nov 21, 2025

Thanks for the great library! I'm really happy with it. However, I came across an edge case which led to a crash in my system.

The issue can be reproduced with the following use case:

  • Default history size: MICRORL_CFG_RING_HISTORY_LEN 64
  • Commands send to history: Display.Backlight(0), Display.Backlight(1), Display.Backlight(2), Display.Backlight(3).
  • Sending the up-key to select the last history entry subsequently crashed my program

Analysis:

  • Contents of the commands don't matter, the error is seemingly triggered by the length of the commands leading to the trailing \0 being the last byte of the buffer.
  • There is an out-of-bounds situation on this variable
    ++idx;                                      /* Move position from `\0` marker */
    
    (see diff)
  • Crash happened here:
    size_t part0 = MICRORL_ARRAYSIZE(rbuf_ptr->ring_buf) - idx;
    memcpy(line_str, rbuf_ptr->ring_buf + idx, part0);
    
    with a negative part0 leading to memory overwrites.

The proposed code changes fix the error but additional consequences are not entirely clear. Feedback always welcome.

Cheers!
Felix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant