fix: partial_sha256_var_end constrained/unconstrained divergence on non-block-aligned messages - #62
Merged
Merged
Conversation
…_end Reproduces noir-lang/noir-library-claude#16: the constrained path of partial_sha256_var_end drops the trailing real_message_size % 64 preimage bytes, diverging from sha256_var. Existing fuzz tests only used 192-byte (block-aligned) inputs, so this case was never exercised. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…224) Adds constrained-vs-unconstrained agreement tests and reference tests for partial_sha256_var_end / partial_sha224_var_end across a range of totals, including non-block-aligned sizes that the existing 192-byte fuzz tests never exercised. Also adds an interstitial agreement test. Non-block-aligned cases currently FAIL (noir-lang/noir-library-claude#16); block-aligned cases and the interstitial step pass. These tests are landed before the fix so the fix can be validated against them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The constrained path processed only the block-aligned `message_size` bytes, so the trailing `real_message_size % BLOCK_SIZE` bytes of the final chunk were dropped and left unconstrained, diverging from the unconstrained oracle and from sha256_var. Process the full chunk (block-aligned part plus the trailing partial block) so the tail is bound, and assert real_message_size >= message_size. Fixes noir-lang/noir-library-claude#16. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ained path partial_sha256_var_end's unconstrained path located the trailing partial block via finalize_last_sha256_block using the global offset (real_message_size / 64) * 64. For a composed hash (interstitial calls followed by end) of a non-block-aligned message, the final chunk is re-based to offset 0, so the global offset points past it — the oracle read out of bounds (or the wrong bytes), diverging from the constrained path. Pass the chunk's local block-aligned offset (message_size) separately from the length (real_message_size) so the tail is read from the correct place. Both runtimes now agree for composed non-block-aligned inputs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nputs When message_size plus the trailing real_message_size % 64 bytes exceeds the array capacity N, the partial-hash API reads past the end of msg. Today the constrained end path silently hashes the missing bytes as zeros (the tests "pass when they should have failed"), while the unconstrained path and the interstitial path abort with a bare "Index out of bounds". These should_fail_with tests expect the call to be rejected with a clear message; they fail until that guard is added. SHA-224 is covered too since it delegates to partial_sha256_var_end. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
partial_sha256_var_end and partial_sha256_var_interstitial never checked their size arguments against the array capacity N. When the block-aligned prefix plus the trailing partial block exceeded N, the constrained end path silently hashed the missing bytes as zeros while the unconstrained and interstitial paths read out of bounds, so the two runtimes diverged on a malformed input. Assert that the accessed region fits within N (message_size + real_message_size % BLOCK_SIZE for end, message_size for interstitial), matching the message_size <= N guard sha256_var already has. Both runtimes now reject the input identically. Note real_message_size itself may exceed N (it counts bytes absorbed by earlier interstitial calls), so only the locally-accessed region is bounded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
This PR also fixes two adjacent partial-hash defects found while writing tests for #16, now tracked as separate findings in the audit repo:
|
asterite
added a commit
that referenced
this pull request
Jul 27, 2026
Re-run extraction with lampe main (919a3871) on the pristine source: the workarounds for reilabs/lampe#263 and #270 are no longer needed, so Extracted/ is now reproducible from a clean checkout. Bump the Lean toolchain to v4.29.1 and lock Lampe/stdlib to the extractor's commit. The extracted test-vector array literals need a deeper elaborator stack (--tstack / maxRecDepth), per the reilabs/lampe#270 resolution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TomAFrench
approved these changes
Jul 28, 2026
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.
Summary
Fixes the partial-hash soundness/correctness bug reported in noir-lang/noir-library-claude#16, and closes two further defects in the same API surface that were uncovered while writing tests for it.
partial_sha256_var_end(and, by delegation,partial_sha224_var_end) computes SHA-256 differently in the constrained (proving) path than in the unconstrained (witness-gen) oracle whenever a message is not a whole number of 64-byte blocks. This PR makes the two runtimes agree, hardens the input contract, and adds broad dual-mode test coverage that the previous 192-byte-only tests were missing.The bugs fixed
1. Constrained path dropped the trailing bytes (issue #16)
The constrained branch called
process_full_blocks(msg, message_size, h)with the block-alignedmessage_size, so the trailingreal_message_size % 64bytes were never absorbed — the final block was empty and those bytes were left as free, unconstrained witness. Result: constrained ≠ unconstrained ≠sha256_var, and a prover could set the dropped bytes to anything while satisfying the circuit (preimage-binding break).Fix: process the full final chunk — the block-aligned prefix plus the trailing partial block — so the tail is bound, and finalize with the real length.
2. Unconstrained oracle read the tail at the wrong offset
The unconstrained helper
finalize_last_sha256_blocklocated the trailing block using the global offset(real_message_size / 64) * 64. For a composed hash (one or moreinterstitialcalls followed byend) of a non-block-aligned message, the final chunk is re-based to offset 0, so the global offset points past it — the oracle read out of bounds. Witness generation would crash or diverge (a liveness failure) for a legitimate multi-chunk hash.Fix: pass the chunk's local block-aligned offset (
message_size) separately from the length (real_message_size), so the tail is read from the correct place in the re-based chunk.3. No bounds check against the array capacity
Neither
partial_sha256_var_endnorpartial_sha256_var_interstitialvalidated their size arguments against the message array capacityN(unlikesha256_var, which assertsmessage_size <= N). When the accessed region exceededN, the constrainedendpath silently hashed the missing bytes as zeros while the unconstrained and interstitial paths read out of bounds — another constrained/unconstrained divergence, this time on malformed input.Fix: assert that the locally-accessed region fits within
N—message_size + real_message_size % BLOCK_SIZE <= Nforend, andmessage_size <= Nforinterstitial. Notereal_message_sizeitself may legitimately exceedN(it counts bytes absorbed by earlierinterstitialcalls that are not present in this chunk), so only the locally-read region is bounded.Also added
assert(real_message_size >= message_size, ...)toend, the missing size relationship noted in the audit.Why the old tests missed all this
Every
fuzz_test_partial_hash*test useddata: [u8; 192], and192 % 64 == 0. With a block-aligned total the dropped block is legitimately empty and the tail offsets coincide, so none of the three defects could manifest.Tests added
sha256_var/sha224_var) forpartial_sha256_var_end, across many non-block-aligned sizes, for both SHA-256 and SHA-224.interstitial+end) tests for non-block-aligned messages — a tail-only final chunk and a block-plus-tail final chunk — asserting constrained == unconstrained == reference.should_fail_withregression tests for out-of-capacity inputs (SHA-256 and SHA-224).The full suite (67 tests) passes under both
nargo test(ACIR / constrained) andnargo test --force-brillig(unconstrained), via./scripts/run.sh.Notes
partial_sha256_var_endis preserved: witness generation keeps its cheaper path rather than going through the constrained lookup-table construction. The oracle bug was fixed in place.mutkeywords.Fixes noir-lang/noir-library-claude#16