Skip to content

Keep the in place from offset within the memory. - #21

Open
deadcaf3 wants to merge 1 commit into
eerimoq:masterfrom
deadcaf3:in-place-from-offset-bounds
Open

deadcaf3 wants to merge 1 commit into
eerimoq:masterfrom
deadcaf3:in-place-from-offset-bounds

Conversation

@deadcaf3

Copy link
Copy Markdown

In an in place patch, the adjustment read from the patch is added to segment.from_offset with no limit, and the result is passed straight to mem_read(). A corrupt patch can:

  • overflow the int after a few large adjustments (undefined behavior),
  • move the offset before the memory, so mem_read() gets a huge address,
  • move it near the end, so a diff reads past the memory.

The sequential patch type has from_seek() for this; the in place type has nothing.

The fix adds one helper, in_place_is_from_range_valid(), and calls it in two places:

  1. Adjustment: reject an adjustment that moves the from offset out of [0, memory_size]. The sum is computed as size_t, so it cannot overflow, and a negative result wraps to a large value that is rejected too.
  2. Diff data: reject a read that would go past memory_size.

Valid patches never get there, since the from data always lies inside the memory. The existing in place tests (foo and micropython, several memory and segment sizes, all compressions) still pass.

The tests are three inline patches in c/tst/test_fuzzer.c, using a small in-memory test_one_in_place() helper. Without the fix:

Test Result without the fix
in_place_from_offset_before_memory mem_read() called at (size_t)-1
in_place_from_read_past_memory succeeds, reading 1 byte past the memory
in_place_from_offset_overflow UBSan: detools.c:2057: signed integer overflow: 2147483636 + 134217727

Found by fuzzing the in place apply path, which tests/fuzzer.c and c/tst/fuzzer_corrupt_patch.c do not reach.

Note: the segment size PR adds the same test_one_in_place() helper. Whichever lands second needs a trivial rebase.

The adjustment read from the patch was added to the from offset without
any limit, so a corrupt patch could overflow the int, or move the
offset before or past the memory and have mem_read() called there.

Reject an adjustment that moves the from offset out of the memory, and
a diff that would read past the end of it.
deadcaf3 added a commit to ultrawidelock/tinyfota that referenced this pull request Oct 21, 2026
- tests/powercut/powercut.c cuts the power at every flash operation of a
  real update between two sample builds (fixtures/), both lost and torn,
  then reboots twice. Every one of the 6,526 runs must end on the target
  image or on the untouched base. `make powercut`.
- tests/fuzz/: libFuzzer targets for the header parsers, the receiver's
  frame and SMP upload paths, and patch application, with seeds from
  gen_seeds.py. `make fuzz` builds them, `make fuzz-run` runs each for
  FUZZ_SECONDS.

Two applier defects the simulator found, fixed in core/dfu_applier.c and
pinned by new cases in tests/host/test_tinyfota.c:

- A read-back padded the write buffer's partial word out to flash, so the
  next write started mid-word and the nRF driver refused it, failing the
  patch with the slot half rewritten. Whole words are flushed now and the
  tail is overlaid on the read; a region that starts mid-word is written
  from the word boundary with 0xff lead bytes.
- A step reached the step log before the buffered bytes it covers were in
  flash, so a cut between the two resumed past bytes that never landed.
  The same held for the final clear. Both now flush first.

Two vendored detools crashes the fuzzers found on a corrupt patch, fixed
in third_party/detools/ and sent upstream (THIRD_PARTY.md):

- segment size 0 divided by zero (eerimoq/detools#20);
- the in-place from offset could overflow an int or reach mem_read()
  outside the memory (eerimoq/detools#21).

All 372 saved crash inputs replay clean.
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