fix(cobs): avoid out-of-bounds read in apex_cobs_decode_framed on all-zero input#1
Open
steps-re wants to merge 1 commit into
Open
Conversation
…-zero input
The leading-delimiter skip loop tested `input[i]` before `i < length`:
while (input[i] == 0x00 && i < length) i++;
Because `&&` short-circuits left-to-right, `input[i]` is read before the
bound is checked. When the input is entirely 0x00 (an idle/low UART line, or
a run of frame delimiters — both plausible on a real wire), `i` advances to
`length` and `input[length]` is read one byte past the buffer before the
loop exits. AddressSanitizer confirms a heap-buffer-overflow READ here.
Swap the operands so the bound is checked first:
while (i < length && input[i] == 0x00) i++;
Behavior is otherwise unchanged: an all-zero buffer is still rejected as
APEX_ERR_MALFORMED, and valid framed messages round-trip as before. The
sibling trailing-delimiter loop is already safe (its index is bounded below
by `i`).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Mike German <mike@stepsventures.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apex_cobs_decode_framed()reads one byte past the end ofinputwhen the buffer is entirely0x00.Details
The leading-delimiter skip loop dereferences
input[i]before the bound check:&&evaluates left-to-right, so on an all-zero bufferiwalks up tolengthandinput[length]is read before the loop terminates. An all-zeros buffer is a realistic input on a UART wire (idle line held low, electrical noise, or a run of the0x00bytes APEX uses as frame delimiters), and this is a public API (apex_cobs.h) consuming bytes straight off the wire, so the out-of-bounds read is reachable from malformed/adversarial input.Fix
Swap the operands so the bound is checked first. No behavior change on any valid input.
Verification
-std=c99 -Wall -Wextra -Wpedantic), no new warnings.lengthzero bytes): before →heap-buffer-overflow READ of size 1in the skip loop; after → no error, returnsAPEX_ERR_MALFORMED.apex_cobs_encode_framed→apex_cobs_decode_framed) under ASan still passes.The sibling trailing-delimiter loop is already safe (bounded below by
i). Note:test_cobs.ccdoesn't currently exercise the_framedhelpers — happy to add aDecodeFramedAllZeroscase if you point me at the gtest vendor path.