Prevent ExtraDataPtr reads beyond payload capacity - #261
Open
drebbe-intrepid wants to merge 8 commits into
Open
drebbe-intrepid wants to merge 8 commits into
drebbe-intrepid wants to merge 8 commits into
Conversation
Reject unsupported objects before reading native message fields or transmitting any part of a batch. Preserve both supported message layouts and cover validation with a hardware-free native stub.
Track payload capacity independently of mutable wire lengths to prevent heap overreads. Validate length changes, reads, and native writes; preserve DLL-owned buffers and handle reentrant integer conversions safely. Fixes #240
Write normalized native scalar fields directly and dispatch deletion before snapshots. Restore state before constructing errors so callback-driven payload replacement cannot resurrect freed pointers.
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.
Fixes #240.
ExtraDataPtrpreviously trusted writable length fields: allocating one byte then settingNumberBytesData = 64exposed 64 bytes. Track private payload capacity for both message types, reject unsafe length/protocol/Data/Header changes, and recheck bounds on reads and before native transmit/CoreMini writes. Received buffers retain DLL ownership with their original reported length as the bound. Correct J1850 CoreMini RX passing the mask pointer twice so both returned messages receive independent bounds.Integer conversions finish before rollback snapshots. Protected scalar fields are assigned directly, subclass deletion dispatch runs before snapshots, and state is restored before allocating validation errors. These prevent reentrant integer conversion, subclass descriptors, warning hooks, or finalizers from resurrecting freed payloads. Protocol and length assignments outside 0..255 now raise instead of warning and truncating; protected native field writes bypass subclass property setters consistently with Data/Header.
Validation: reproduced the original overread without printing heap contents; rebuilt on Windows x64 / CPython 3.14.5;
pytest tests/ -qpassed all 79 tests. Regressions cover both message types, all four packed-length protocols, direct descriptor bypass, reentrant conversions, warning hooks, six subprocess subclass setter/deleter cases, replacement, and a mock DLL for receive/CoreMini Tx and RX message/mask/transmit. No hardware accessed. Diff whitespace checks passed. Cross-platform CI remains pending.Independent critic
/root/criticand builder explicitly agree with the initial reviewed diff505231054f90587cee55f59aafdc42ccfe523e08and renewed follow-up diff5a8b786ae72494eb944450e4478bd2fe6fa95217against471d658a1da3b460010fac2d8e55533e61f58b3f. Final critic: “Subclass callback and rollback hazards are resolved. No blocking findings remain. Reviewed source/test hashes match; builder reports successful rebuild and 79 passing tests.”Dependency integration
Merged reviewed fixes from PRs252,255,256. Independent aggregate critic and coordinator agree with head7f5bdf05ef3da822a84a06a18081d9f173960943; clean build and117tests pass on Windows CPython3.14.5. Final-head CI pending.