Size the sanitised fixtures to the length under test #56
Loading…
Reference in a new issue
No description provided.
Delete branch "test/uf-14-heap-sized-manifest-fixtures"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Tests that hand a
.rodataliteral or an oversized staging buffer to a functiontaking a pointer and a length put ASan's redzone past the end of the object, not
at the bound the callee was given. An over-read inside the declared length reads
valid memory and the suite stays green. The manifest tests, the session
credential tests and the digest tests all had that shape.
Each now copies into a heap buffer sized exactly to the length under test, via
one helper per file.
Proven by reintroducing a bound bug in each of the three parsers and checking the
suite before and after the fixture change:
end_read inreadString: passed before, ASanheap-buffer-overflow after
terminatorAfterwith itsat < lengthguard dropped: passed before, ASanheap-buffer-overflow after
Sha256::updatecopying one byte more thantake: passed before, ASanheap-buffer-overflow after
Worth knowing: the actual CCS-UF-5 over-read (
480c10b, the manifest keycomparison walking
wpast a name's NUL) already fails the suite today as anASan global-buffer-overflow, because the names it over-reads are instrumented
globals. The gap this closes is the JSON body's own bound, not that one.
Tests only — no production code, specification or vectors touched, and the
test_mcp2515_txleak suppression is unchanged. Fullpio test -e native,both node environments and
host_simall build and pass.CCS-UF-14.
One minor point, on the zero-length case. Nothing blocking.
@ -11,0 +11,4 @@// Heap-allocated and sized exactly to the length under test, so the sanitiser's// redzone sits where update's bound is. A larger fixture, or a pointer into the// middle of one, would let a read past that bound go unreported.void feed(ctrl::Sha256& hash, const uint8_t* from, uint32_t length) {At length 0 the redzone is not at the bound: ASan gives
new uint8_t[0]a one-byte addressable chunk (offset 0 unpoisoned, offset 1 onwards poisoned), so a one-past read at index 0 is still unreported. That hitsdigestOf("")intest_the_empty_input, andparseExactly("", ...)in the manifest tests (harmless there —parseFirmwareManifestreturns onlength == 0before reading). The comment states the guarantee unconditionally; worth qualifying it, since there is no fixture fix.ffa15bfis comments only and the new wording holds at every length, zero included. Nothing further to act on.