Skip to content

Prevent ExtraDataPtr reads beyond payload capacity - #261

Open
drebbe-intrepid wants to merge 8 commits into
masterfrom
codex/fix-240-payload-capacity
Open

drebbe-intrepid wants to merge 8 commits into
masterfrom
codex/fix-240-payload-capacity

Conversation

@drebbe-intrepid

@drebbe-intrepid drebbe-intrepid commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Fixes #240.

ExtraDataPtr previously trusted writable length fields: allocating one byte then setting NumberBytesData = 64 exposed 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/ -q passed 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/critic and builder explicitly agree with the initial reviewed diff 505231054f90587cee55f59aafdc42ccfe523e08 and renewed follow-up diff 5a8b786ae72494eb944450e4478bd2fe6fa95217 against 471d658a1da3b460010fac2d8e55533e61f58b3f. 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.

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.
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.

[P1] Writable message length fields allow ExtraDataPtr to read past its allocation

1 participant