From a4855a7691286459be652276bbffdd4522a3f0e6 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Thu, 10 Sep 2026 20:37:48 +0200 Subject: [PATCH] ceremony: accept JSON whitespace in attested fields Preserve original transcript offsets and bearer-only commitments while accepting JSON whitespace around colons and before integer terminators. Assisted-by: GPT-6 Signed-off-by: Wondertan --- .../ceremony/CeremonyAttestation.sol | 30 ++++++---- .../contracts/ceremony/CeremonyFields.sol | 59 ++++++++++++------- .../ceremony/TlsNotaryVerifierBase.sol | 32 +--------- .../ceremony/test/CeremonyAttestation.t.sol | 46 +++++++++++++++ .../ceremony/test/CeremonyFields.t.sol | 16 ++++- .../test/GitHubPlatformVerifier.t.sol | 11 +++- .../ceremony/test/XPlatformVerifier.t.sol | 2 +- 7 files changed, 129 insertions(+), 67 deletions(-) diff --git a/solidity/contracts/ceremony/CeremonyAttestation.sol b/solidity/contracts/ceremony/CeremonyAttestation.sol index 74249dc..2e4e6fe 100644 --- a/solidity/contracts/ceremony/CeremonyAttestation.sol +++ b/solidity/contracts/ceremony/CeremonyAttestation.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; +import {CeremonyFields} from "./CeremonyFields.sol"; + /// @title CeremonyAttestation /// @notice Decoder for the attested-data layout the launch profiles pin. /// @dev THE LAYOUT IS THE PROFILE'S, NOT THE SPECIFICATION'S. REQ-COMMON-18 @@ -142,7 +144,7 @@ library CeremonyAttestation { /// @dev The normalized, line-anchored needle REQ-COMMON-39 counts. bytes internal constant AUTHORIZATION_NEEDLE = "\r\nauthorization:bearer"; - /// @notice The one commitment framed by exactly these revealed bytes. + /// @notice The one commitment framed by a revealed JSON string prefix and closing quote. /// /// @dev For a direction that is NOT exactly covered, where several ranges /// are hidden and only the anchors around one of them are revealed. @@ -158,22 +160,26 @@ library CeremonyAttestation { /// /// Exactly one commitment may carry the framing. Two would leave /// nothing to say which the circuit opened. - function requireFramedCommitment(DirectionBlock memory block_, bytes memory prefix, bytes memory suffix) + function requireJsonStringCommitment(DirectionBlock memory block_, string memory name) internal pure returns (RangeCommitment memory framed) { + (uint256 count,) = CeremonyFields.jsonPrefix(concatRevealed(block_), name, true); + if (count > 1) revert AmbiguousFraming(); uint256 found = type(uint256).max; - for (uint256 i = 0; i < block_.commitments.length; ++i) { - RangeCommitment memory c = block_.commitments[i]; - if (c.start < prefix.length) continue; - bytes memory before_ = _revealedSlice(block_, c.start - uint32(prefix.length), c.start); - if (keccak256(before_) != keccak256(prefix)) continue; - bytes memory after_ = _revealedSlice(block_, c.end, c.end + uint32(suffix.length)); - if (keccak256(after_) != keccak256(suffix)) continue; - - if (found != type(uint256).max) revert AmbiguousFraming(); - found = i; + for (uint256 i = 0; i < block_.revealed.length; ++i) { + RevealedRange memory anchor = block_.revealed[i]; + (uint256 matches, uint256 quote) = CeremonyFields.jsonPrefix(anchor.value, name, true); + if (matches != 1 || quote + 1 != anchor.value.length) continue; + for (uint256 j = 0; j < block_.commitments.length; ++j) { + RangeCommitment memory c = block_.commitments[j]; + if (uint256(anchor.start) + anchor.value.length != c.start) continue; + bytes memory after_ = _revealedSlice(block_, c.end, c.end + 1); + if (after_.length != 1 || after_[0] != '"') continue; + if (found != type(uint256).max) revert AmbiguousFraming(); + found = j; + } } if (found == type(uint256).max) revert NoFramedCommitment(); return block_.commitments[found]; diff --git a/solidity/contracts/ceremony/CeremonyFields.sol b/solidity/contracts/ceremony/CeremonyFields.sol index 98e9872..559df23 100644 --- a/solidity/contracts/ceremony/CeremonyFields.sol +++ b/solidity/contracts/ceremony/CeremonyFields.sol @@ -51,12 +51,10 @@ library CeremonyFields { pure returns (Found found, bytes memory value) { - bytes memory needle = abi.encodePacked('"', name, '":"'); - uint256 at; - (found, at) = _findUnique(data, needle); - if (found != Found.One) return (found, ""); - - at += needle.length; + (uint256 count, uint256 at) = jsonPrefix(data, name, true); + if (count == 0) return (Found.None, ""); + if (count > 1) return (Found.Several, ""); + ++at; uint256 end = at; while (end < data.length && data[end] != '"') { ++end; @@ -88,20 +86,20 @@ library CeremonyFields { pure returns (Found found, bytes memory digits) { - bytes memory needle = abi.encodePacked('"', name, '":'); - uint256 at; - (found, at) = _findUnique(data, needle); - if (found != Found.One) return (found, ""); - - at += needle.length; + (uint256 count, uint256 at) = jsonPrefix(data, name, false); + if (count == 0) return (Found.None, ""); + if (count > 1) return (Found.Several, ""); uint256 end = at; while (end < data.length && data[end] >= "0" && data[end] <= "9") { ++end; } if (end == at) revert NoncanonicalInteger(name); if (end - at > 1 && data[at] == "0") revert NoncanonicalInteger(name); - if (end == data.length) return (Found.None, ""); - if (data[end] != "," && data[end] != "}") revert BadIntegerTerminator(name, data[end]); + uint256 terminator = _skipWhitespace(data, end); + if (terminator == data.length) return (Found.None, ""); + if (data[terminator] != "," && data[terminator] != "}") { + revert BadIntegerTerminator(name, data[terminator]); + } digits = new bytes(end - at); for (uint256 i = 0; i < digits.length; ++i) { @@ -110,15 +108,32 @@ library CeremonyFields { return (Found.One, digits); } - function _findUnique(bytes memory data, bytes memory needle) private pure returns (Found found, uint256 at) { - uint256 hit = type(uint256).max; - for (uint256 i = 0; i + needle.length <= data.length; ++i) { - if (!_matchesAt(data, needle, i)) continue; - if (hit != type(uint256).max) return (Found.Several, 0); - hit = i; + /// @notice Count field prefixes and return the last value offset (opening quote for strings). + /// @dev Only JSON whitespace is skipped. Callers read a value only when count is one. + function jsonPrefix(bytes memory data, string memory name, bool quoted) + internal + pure + returns (uint256 count, uint256 at) + { + bytes memory key = abi.encodePacked('"', name, '"'); + for (uint256 i = 0; i + key.length <= data.length; ++i) { + if (!_matchesAt(data, key, i)) continue; + uint256 colon = _skipWhitespace(data, i + key.length); + if (colon == data.length || data[colon] != ":") continue; + uint256 value = _skipWhitespace(data, colon + 1); + if (quoted && (value == data.length || data[value] != '"')) continue; + ++count; + at = value; + } + } + + function _skipWhitespace(bytes memory data, uint256 at) private pure returns (uint256) { + while (at < data.length) { + bytes1 c = data[at]; + if (c != 0x20 && c != 0x09 && c != 0x0a && c != 0x0d) break; + ++at; } - if (hit == type(uint256).max) return (Found.None, 0); - return (Found.One, hit); + return at; } /// @notice The value of `name=value` in an `application/x-www-form-urlencoded` diff --git a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol index f1a24a5..97a2a01 100644 --- a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol +++ b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol @@ -43,9 +43,6 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa /// and no profile lists it. bytes private constant LENGTH_HEADER = "content-length: "; - bytes internal constant ACCESS_TOKEN_PREFIX = '"access_token":"'; - bytes internal constant ACCESS_TOKEN_SUFFIX = '"'; - /// @notice What a TLSNotary profile decodes from its payload. /// /// @dev `abi.encode` of this struct is the payload for `x/v1` and @@ -162,29 +159,6 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa virtual returns (string memory idField, IdShape idShape, string memory handleField); - /// @dev How many times a field's full delimiter appears across the whole - /// revealed set, seams included. - /// - /// Counting and reading want opposite things. A READ must stay inside - /// one authenticated range, or a prover splices a document that never - /// crossed the wire. A COUNT must not miss, or a prover cuts a range - /// through a second delimiter and the duplicate REQ-COMMON-19A exists - /// to reject becomes invisible to it. So the value is read per range - /// and the occurrences are counted over the concatenation -- where a - /// seam can only over-count, which fails closed. - function _delimiterCount(bytes memory joined, bytes memory delimiter) private pure returns (uint256 count) { - for (uint256 i = 0; i + delimiter.length <= joined.length; ++i) { - bool hit = true; - for (uint256 j = 0; j < delimiter.length; ++j) { - if (joined[i + j] != delimiter[j]) { - hit = false; - break; - } - } - if (hit) ++count; - } - } - /// @dev Find a JSON string field in exactly one revealed range. /// /// Reading from a concatenation of the revealed ranges is what this @@ -215,7 +189,7 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa if (matches != 1) revert FieldNotUnique(name, matches); // And the delimiter appears once across the whole revealed set, so a // second copy cannot hide under a range boundary. - uint256 seen = _delimiterCount(joined, abi.encodePacked('"', name, '":"')); + (uint256 seen,) = CeremonyFields.jsonPrefix(joined, name, true); if (seen != 1) revert FieldNotUnique(name, seen); } @@ -235,7 +209,7 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa } } if (matches != 1) revert FieldNotUnique(name, matches); - uint256 seen = _delimiterCount(joined, abi.encodePacked('"', name, '":')); + (uint256 seen,) = CeremonyFields.jsonPrefix(joined, name, false); if (seen != 1) revert FieldNotUnique(name, seen); } @@ -350,7 +324,7 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa // The bearer is identified by its framing, not by being the only // commitment: the response hides every other byte behind one of its own. CeremonyAttestation.RangeCommitment memory bearer = - CeremonyAttestation.requireFramedCommitment(data.received, ACCESS_TOKEN_PREFIX, ACCESS_TOKEN_SUFFIX); + CeremonyAttestation.requireJsonStringCommitment(data.received, "access_token"); tokenCommitment = bearer.commitment; // The token attestation is the one-time PKCE and digest binding, so it diff --git a/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol b/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol index ab6a885..245024e 100644 --- a/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol +++ b/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol @@ -30,6 +30,52 @@ contract CeremonyAttestationTest is Test { return CeremonyAttestation.decode(data); } + function framed(CeremonyAttestation.DirectionBlock memory block_) external pure returns (bytes32) { + return CeremonyAttestation.requireJsonStringCommitment(block_, "access_token").commitment; + } + + function test_spacedBearerFramingKeepsWhitespaceRevealed() public { + bytes memory prefix = bytes('"access_token" \t: \r\n"'); + uint32 start = uint32(prefix.length); + CeremonyAttestation.DirectionBlock memory block_; + block_.revealed = new CeremonyAttestation.RevealedRange[](2); + block_.revealed[0] = CeremonyAttestation.RevealedRange({start: 0, end: start, value: prefix}); + block_.revealed[1] = CeremonyAttestation.RevealedRange({start: start + 5, end: start + 6, value: '"'}); + block_.commitments = new CeremonyAttestation.RangeCommitment[](1); + block_.commitments[0] = + CeremonyAttestation.RangeCommitment({start: start, end: start + 5, commitment: bytes32(uint256(7))}); + assertEq(this.framed(block_), bytes32(uint256(7))); + block_.commitments[0].start = start + 1; + vm.expectRevert(CeremonyAttestation.NoFramedCommitment.selector); + this.framed(block_); + block_.commitments[0].start = start; + // A second prefix is ambiguous even without a second framed commitment. + block_.revealed[1].value = bytes('" "access_token":"'); + block_.revealed[1].end = block_.revealed[1].start + uint32(block_.revealed[1].value.length); + vm.expectRevert(CeremonyAttestation.AmbiguousFraming.selector); + this.framed(block_); + } + + function test_refusesASpacedBearerPrefixSplitAcrossDisjointRanges() public { + bytes memory first = bytes('"access_token" '); + bytes memory second = bytes(': \t"'); + uint32 split = uint32(first.length); + uint32 bearer = split + 1 + uint32(second.length); + CeremonyAttestation.DirectionBlock memory block_; + block_.revealed = new CeremonyAttestation.RevealedRange[](3); + block_.revealed[0] = CeremonyAttestation.RevealedRange({start: 0, end: split, value: first}); + block_.revealed[1] = CeremonyAttestation.RevealedRange({start: split + 1, end: bearer, value: second}); + block_.revealed[2] = CeremonyAttestation.RevealedRange({start: bearer + 5, end: bearer + 6, value: '"'}); + block_.commitments = new CeremonyAttestation.RangeCommitment[](2); + block_.commitments[0] = + CeremonyAttestation.RangeCommitment({start: split, end: split + 1, commitment: bytes32(uint256(8))}); + block_.commitments[1] = + CeremonyAttestation.RangeCommitment({start: bearer, end: bearer + 5, commitment: bytes32(uint256(7))}); + CeremonyAttestation.requireExactCoverage(block_, bearer + 6); + vm.expectRevert(CeremonyAttestation.NoFramedCommitment.selector); + this.framed(block_); + } + function test_decodesTheRustEncoderOutput() public view { CeremonyAttestation.AttestedData memory a = this.decode(FIXTURE); diff --git a/solidity/contracts/ceremony/test/CeremonyFields.t.sol b/solidity/contracts/ceremony/test/CeremonyFields.t.sol index e7689b5..8eea1c8 100644 --- a/solidity/contracts/ceremony/test/CeremonyFields.t.sol +++ b/solidity/contracts/ceremony/test/CeremonyFields.t.sol @@ -80,6 +80,18 @@ contract CeremonyFieldsTest is Test { assertEq(this.jsonString(bytes('{"username":""}'), "username").length, 0); } + function test_jsonWhitespaceAndMixedDuplicateSpellings() public { + bytes memory body = bytes('{"login" \t:\r\n "alice", "id"\n : \t123 \r\n}'); + assertEq(string(this.jsonString(body, "login")), "alice"); + assertEq(string(this.jsonInteger(body, "id")), "123"); + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.AmbiguousField.selector, "login")); + this.jsonString(bytes('{"login":"alice","login" : "bob"}'), "login"); + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.AmbiguousField.selector, "id")); + this.jsonInteger(bytes('{"id":123,"id" : nope}'), "id"); + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.FieldNotFound.selector, "login")); + this.jsonString(bytes.concat(bytes('{"login":'), hex"0b", bytes('"alice"}')), "login"); + } + // ─── JSON integers ────────────────────────────────────────────── /// @dev GitHub's `/user.id` is a bare integer, and the terminator is what @@ -91,10 +103,10 @@ contract CeremonyFieldsTest is Test { } function test_refusesAnyOtherTerminator() public { - // A space would let `123 456` read as `123`. Casting the literal to + // Whitespace must still be followed by a structural byte, not more digits. Casting to // bytes1 is safe: one longer than a byte would not compile. // forge-lint: disable-next-line(unsafe-typecast) - vm.expectRevert(abi.encodeWithSelector(CeremonyFields.BadIntegerTerminator.selector, "id", bytes1(" "))); + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.BadIntegerTerminator.selector, "id", bytes1("4"))); this.jsonInteger(bytes('{"id":123 456}'), "id"); } diff --git a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol index 0868daf..3e47354 100644 --- a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol @@ -277,11 +277,20 @@ contract GitHubPlatformVerifierTest is Test { /// rather than a prefix of a longer one. function test_rejectsAnIdWithoutAStructuralTerminator() public { TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); - s.identitySession = _identity('{"login":"octocat","id":583231 }', CeremonyProfile.AUTHORITY_GITHUB_API); + s.identitySession = _identity('{"login":"octocat","id":583231 4}', CeremonyProfile.AUTHORITY_GITHUB_API); vm.expectPartialRevert(CeremonyFields.BadIntegerTerminator.selector); this.run{value: quote}(s); } + function test_readsPrettyPrintedIdentity() public { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = + _identity('{\n "login" : "octocat",\n "id": 583231 \n}', CeremonyProfile.AUTHORITY_GITHUB_API); + ICeremony.VerifiedClaim memory result = this.run{value: quote}(s); + assertEq(result.userId, "583231"); + assertEq(result.handle, "octocat"); + } + function test_rejectsANoncanonicalId() public { TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); s.identitySession = _identity('{"login":"octocat","id":007}', CeremonyProfile.AUTHORITY_GITHUB_API); diff --git a/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol index ae35ce5..4e6443b 100644 --- a/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol @@ -693,7 +693,7 @@ contract XPlatformVerifierTest is Test { // Joined: ...,"username":"alice","username":"mallory"} -- two members, // and the boundary falls through the second one's delimiter. s.identitySession = _splitIdentityAttestation( - 'HTTP/1.1 200 OK\r\n\r\n{"id":"2244994945","username":"alice","userna', 'me":"mallory"}' + 'HTTP/1.1 200 OK\r\n\r\n{"id":"2244994945","username":"alice","userna', 'me" \t: \n"mallory"}' ); vm.expectPartialRevert(TlsNotaryVerifierBase.FieldNotUnique.selector); this.run{value: quote}(s);