fix(p2p): bound declared contract-class bytecode length before allocating (A-1258)#24213
Merged
PhilWindle merged 2 commits intoJun 22, 2026
Conversation
…ting (A-1258) DataTxValidator decodes contract-class publication logs before proof validation. bufferFromFields trusted the declared byte-length field and called Buffer.alloc(byteLength) even when it far exceeded the payload actually present, so a malformed class-publication tx could force attacker-chosen multi-MiB allocations on every node that validates the gossiped tx (req/resp, block proposal, RPC, gossip) before the proof validator rejects it. Add an optional maxByteLength guard to bufferFromFields (preserving the documented blob-reconstruction padding for callers that don't pass it); ContractClassPublishedEvent.fromLog passes the fixed log's payload capacity, so an over-declared length throws before allocation. DataTxValidator already wraps fromLog in try/catch, so the throw becomes a clean pre-proof rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
alexghr
reviewed
Jun 22, 2026
| // The fixed-size class log can hold at most this many packed-bytecode bytes (every payload field | ||
| // carries 31 bytes). Bound the declared length to it so a malformed log can't declare a multi-MiB | ||
| // length backed by a tiny payload and force a large allocation during early (pre-proof) validation. | ||
| const maxByteLength = (bytecodeFields.length - 1) * (Fr.SIZE_IN_BYTES - 1); |
Contributor
There was a problem hiding this comment.
can byecodeFields end up being an empty buffer?
Collaborator
Author
There was a problem hiding this comment.
Shouldn't be, but I added a guard against it
…258) Defensive guard from PR review: an empty fields array would destructure length to undefined and throw on length.toNumber(). A valid bufferAsFields output always carries the leading byte-length field, so empty encodes no data — return an empty buffer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alexghr
approved these changes
Jun 22, 2026
PhilWindle
deleted the
phil/a-1258-datatxvalidator-allocates-attacker-declared-bytecode-length
branch
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
3 participants
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 A-1258.
Problem
DataTxValidatordecodes contract-class publication logs before proof validation (data validation runs ahead of proof validation on the req/resp, block-proposal, RPC, and gossip paths). The decode trusts the declared public-bytecode length field:bufferFromFields(stdlib/src/abi/buffer.ts) reads the declaredbyteLengthand, when the payload is shorter, doesBuffer.alloc(byteLength)— trusting the declared length over the bytes actually present.ContractClassPublishedEvent.fromLogdecodes the bytecode viabufferFromFields, andDataTxValidator.#hasCorrectContractClassIdscallsfromLogbefore any class-id / proof rejection.A contract-class log is a fixed
CONTRACT_CLASS_LOG_SIZE_IN_FIELDSarray, so the real packed bytecode is capped at ~93 KiB. But the declared length is unbounded: a ~96 KiB log declaring 16 MiB forces a 16 MiB allocation on every node that validates the tx, before the proof check rejects it — a memory-pressure / validation-amplification path.Fix
maxByteLengthparameter tobufferFromFields; when provided, it throws (before allocating) if the declared length exceeds it. Callers that don't pass it keep the documented blob-reconstruction padding behaviour unchanged.fromLogpasses the fixed log's payload capacity (payloadFields × 31), so an over-declared length is rejected before allocation.DataTxValidatoralready wrapsfromLogin try/catch, so the throw becomes a clean pre-proofTX_ERROR_MALFORMED_CONTRACT_CLASS_LOGrejection.Tests
data_validator.test.ts: a log declaring a 16 MiB length is rejected, andBuffer.allocis never called with the declared size (test time drops from ~1.2s to ~40ms — no large allocation). Run against the pre-fix compiled code, the allocation still happened (red); green after the fix.contract_class_published_event.test.ts"fits a max-size public bytecode" and thebuffer.test.tspadding tests confirm legitimate max-size bytecode and the padding contract are unaffected.