fix: hold string claims and exp to REQ-COMMON-19 / REQ-COMMON-19D - #4
Merged
Merged
Conversation
Two spec-compliance gaps found in review of #1, both pre-existing on main. REQ-COMMON-19 requires a string claim's match to cover the closing quote AND the following structural byte. Only email_verified and exp asserted their trailing byte; email, nonce, sub, iss and aud stopped at the closing quote, so a claim-shaped substring inside a longer signed string value would have matched -- JSON escaping of attacker-typed content was the only thing in the way. Every claim now ends at `,` or `}` via one shared assert_structural(), and the section-9 bounds grow by one byte so the trailing byte itself must lie inside the authenticated payload length (REQ-COMMON-19B), not in prover-controlled zero padding. REQ-COMMON-19D pins integers to canonical 0|[1-9][0-9]* and lists leading zeros among the rejects; the exp digit-run check accepted them, so a leading-zero rendering was a second signed encoding of the same u64. A multi-digit exp may no longer start with '0'; the lone "0" stays legal. Negative tests cover both: a claim-shaped substring whose apparent closing quote is followed by ordinary string content now fails, as does a leading-zero exp; companion positive tests pin the accepted shapes. The added constraints move jwt_email from 179,367 to 179,413 gates (+46, 0.03 %) and change its vk_hash from 0x24db903f...88a5a69f to 0x1596b429...2af7a27c; the on-chain verifier regenerates with the next release. bearer-link and dyaka-noir-token artifacts are byte-identical. Assisted-by: Claude Fable 5 Signed-off-by: xgreenx <xgreenx9999@gmail.com>
Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
SupremaLex
added a commit
that referenced
this pull request
Aug 24, 2026
) * fix: restore the REQ-COMMON-19 / REQ-COMMON-19D constraints These were written by xgreenx in review of #1, merged into that branch as #4, and then lost: I rebased the branch onto main and force-pushed, so the merge commit carrying them was discarded and #1 landed without them. Restoring the patch verbatim onto the renamed path. REQ-COMMON-19 requires a string claim's match to cover the closing quote AND the following structural byte. Only `email_verified` and `exp` asserted their trailing byte; `email`, `nonce`, `sub`, `iss` and `aud` stopped at the closing quote, so a claim-shaped substring inside a longer signed string value would have matched -- JSON escaping of attacker-typed content was the only thing in the way. Every claim now ends at `,` or `}` through one shared `assert_structural()`, and the section-9 bounds grow by one byte so the trailing byte itself must lie inside the authenticated payload length (REQ-COMMON-19B) rather than in prover-controlled zero padding. REQ-COMMON-19D pins integers to canonical `0|[1-9][0-9]*` and lists leading zeros among the rejects; the `exp` digit-run check accepted them, so a leading-zero rendering was a second signed encoding of the same u64. A multi-digit `exp` may no longer start with `0`; the lone `0` stays legal. The rebuilt key is `0x1596b4295a9471df3d620bb3a94c970dd695a0617117ed4d0c9190ad2af7a27c`, which is the value xgreenx's original commit predicted -- an independent check that the restore is byte-exact. Tests go 3 to 7; `bearer-link` is unchanged. Still missing from main, and not in this commit: the bb 5.2.0 / nargo 1.0.0-beta.25 toolchain bump, lost the same way. It moves every key again and needs that toolchain installed to verify, so it goes on its own. Co-authored-by: xgreenx <xgreenx9999@gmail.com> Signed-off-by: SupremaLex <georglutsenko@gmail.com> * chore: restore the bb 5.2.0 / nargo 1.0.0-beta.25 toolchain bump Wondertan's, lost in the same force-push as the constraints in the commit before this one. Restoring it here rather than after the release, because it moves every verification key and releasing without it means re-cutting. Rebuilt and verified under the restored pins, with x-token already gone: bearer-link 0x1d161afb536683d31a3e426db0feaa30de8be89cc45510579f361266c20e078f oidc-google 0x1b50bbf6d8ea6efc7ecc2547c25b285511704a10d9247466e84045095d9c3f77 Both keys move: the toolchain is what derives them, which is the whole reason the pins are a single source of truth. Downstream verifiers regenerate from these, so libid-contracts rolls its committed Solidity together with the tag that carries them. bearer_link 12 tests, oidc_google 7, `nargo fmt --check` clean on both under beta.25. Co-authored-by: Wondertan <hlibwondertan@gmail.com> Signed-off-by: SupremaLex <georglutsenko@gmail.com> --------- Signed-off-by: SupremaLex <georglutsenko@gmail.com> Co-authored-by: xgreenx <xgreenx9999@gmail.com> Co-authored-by: Wondertan <hlibwondertan@gmail.com>
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
2 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.
Closes out the two findings from the review of #1 — both pre-existing on
main, both defense-in-depth misses in
circuits/jwt_email/src/main.nr.Targets the
feat/ceremony-circuitsbranch so it lands as part of #1'shistory; the branch is #1's head plus this one commit.
REQ-COMMON-19 — trailing structural byte on string claims
The spec requires the circuit to "assert the full
"field":"delimiter atthat offset, the value bytes, the closing quote, and the following
structural byte fixed by the profile". Only
email_verifiedandexpasserted their trailing byte; the five string claims —
email,nonce,sub,iss,aud— stopped at the closing quote, so a claim-shapedsubstring inside a longer signed string value would have satisfied the
match, with JSON escaping of attacker-typed content the only thing in the
way.
Every claim now asserts the byte after its match is
,or}through oneshared
assert_structural()(the two existing inline checks fold into it),and the section-9 offset bounds each grow by one byte so the trailing byte
itself must sit inside the authenticated payload length per REQ-COMMON-19B —
without that, the new assertion could be satisfied from prover-controlled
zero padding.
REQ-COMMON-19D — canonical
expThe spec pins integers to canonical
0|[1-9][0-9]*and explicitly listsleading-zero among the rejects. The digit-run check accepted any digits, so
"exp":0123…was a second signed encoding of the same u64. A multi-digitexpmay no longer begin with'0'; the lone"0"remains legal.Tests
Four new tests (jwt_email 3 → 7, repo 24 → 28): a claim-shaped substring
whose apparent closing quote is followed by ordinary string content fails
with the new assertion after passing every pre-existing check, and a
leading-zero
expfails; companion positive tests pin,/}continuations and the two canonical
expshapes (ten-digit timestamp,lone
"0").Artifact impact (pinned toolchain: nargo 1.0.0-beta.20, bb 5.0.0-nightly.20260324)
0x24db903f…88a5a69f0x1596b429…2af7a27cThe README's recorded jwt_email vk_hash is updated accordingly (the
2026-08-12 reproducibility note now dates its hash and records the current
one), and the stale "jwt_email has no tests" comment in ci.yml is corrected.
nargo fmt --check,nargo test(28/28) andscripts/build.shall passlocally under the pinned toolchain.
🤖 Generated with Claude Code
Reopened from #3, which was squash-merged into
feat/ceremony-circuitsrather than left standing as a reviewable diff on top of #1. That merge (effeb69) has been rolled back;feat/ceremony-circuitsis restored to5b1a0bdand this PR carries the identical change for review.