From 1dd67f6116dadbcfbfd30aa8048a6de5f5a6b8ea Mon Sep 17 00:00:00 2001 From: xgreenx Date: Mon, 5 Oct 2026 14:43:44 +0100 Subject: [PATCH 1/2] fix(ceremony): hold X and GitHub ids to the REQ-PLAT-06 grammar REQ-PLAT-06 requires an X or GitHub id to match ^[1-9][0-9]{0,19}$ with a value at most 2^64 - 1. The X reader returned any string between the quotes and the GitHub reader accepted 0 and ids past uint64, so the registry could record an id the browser and the spec refuse. Both now revert with NoncanonicalUserId. The reference copy takes the rule too, so the differential tests still hold. Costs 1.4k gas per GitHub claim and 2.8k per X claim. Assisted-by: Claude Opus 5.5 Signed-off-by: xgreenx --- .../contracts/ceremony/CeremonyFields.sol | 25 ++++++++++++ .../ceremony/TlsNotaryVerifierBase.sol | 15 ++++--- .../ceremony/test/CeremonyFields.t.sol | 40 ++++++++++++++++++- .../test/GitHubPlatformVerifier.t.sol | 20 ++++++++++ .../ceremony/test/TranscriptEquivalence.t.sol | 22 +++++++++- .../ceremony/test/TranscriptReference.sol | 31 ++++++++++---- .../ceremony/test/XPlatformVerifier.t.sol | 18 +++++++++ solidity/snapshots/claim-verification.json | 4 +- 8 files changed, 157 insertions(+), 18 deletions(-) diff --git a/solidity/contracts/ceremony/CeremonyFields.sol b/solidity/contracts/ceremony/CeremonyFields.sol index 588105c9..997023db 100644 --- a/solidity/contracts/ceremony/CeremonyFields.sol +++ b/solidity/contracts/ceremony/CeremonyFields.sol @@ -567,4 +567,29 @@ library CeremonyFields { } return true; } + + /// @notice Whether `value` is the canonical decimal spelling of a nonzero + /// unsigned 64-bit integer: `^[1-9][0-9]{0,19}$`, at most + /// `2^64 - 1`. + /// + /// @dev REQ-PLAT-06, the grammar of an X or GitHub `userId`: every role + /// must record one account under the same bytes. + function isCanonicalNonzeroUint64(bytes memory value) internal pure returns (bool) { + uint256 n = value.length; + if (n == 0 || n > 20) return false; + // Twenty bytes fit in one word, read from its most significant byte. + uint256 w = _word(value, 0); + if (w >> 248 == 0x30) return false; + uint256 number; + unchecked { + for (uint256 i = 0; i < n; ++i) { + // A byte below `0` wraps past 9, so one comparison bounds both ends. + uint256 digit = ((w >> (248 - 8 * i)) & 0xff) - 0x30; + if (digit > 9) return false; + // At most twenty digits: no product here can overflow. + number = number * 10 + digit; + } + } + return number <= type(uint64).max; + } } diff --git a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol index 47237f78..1a37f796 100644 --- a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol +++ b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol @@ -93,6 +93,9 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa error ClientIdentifierNotSerializerSafe(bytes found); /// @dev A field was found in no revealed range, or in more than one. error FieldNotUnique(string name, uint256 rangesMatching); + /// @dev The id read is not a nonzero unsigned 64-bit integer in its + /// canonical decimal spelling (REQ-PLAT-06). + error NoncanonicalUserId(bytes found); /// @dev The first revealed range does not begin the transcript, so nothing /// says the bytes read as a request line ARE the request line. error RequestLineNotAtOrigin(uint32 start); @@ -435,11 +438,13 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa } bytes memory joined = CeremonyFields.normalizeJsonBytes(CeremonyAttestation.concatRevealed(data.received)); (string memory idField, IdShape idShape, string memory handleField) = _identityFields(); - userId = string( - idShape == IdShape.JsonString - ? _uniqueJsonString(ranges, joined, idField) - : _uniqueJsonInteger(ranges, joined, idField) - ); + bytes memory id = idShape == IdShape.JsonString + ? _uniqueJsonString(ranges, joined, idField) + : _uniqueJsonInteger(ranges, joined, idField); + // The readers hold the id to its JSON syntax only: X's quotes admit + // any string, GitHub's digits a lone `0` and any length. + if (!CeremonyFields.isCanonicalNonzeroUint64(id)) revert NoncanonicalUserId(id); + userId = string(id); handle = string(_uniqueJsonString(ranges, joined, handleField)); } diff --git a/solidity/contracts/ceremony/test/CeremonyFields.t.sol b/solidity/contracts/ceremony/test/CeremonyFields.t.sol index 6e173de6..e6788ec7 100644 --- a/solidity/contracts/ceremony/test/CeremonyFields.t.sol +++ b/solidity/contracts/ceremony/test/CeremonyFields.t.sol @@ -122,8 +122,12 @@ contract CeremonyFieldsTest is Test { this.jsonInteger(bytes('{"id":1.5}'), "id"); } - function test_zeroIsCanonical() public view { - assertEq(string(this.jsonInteger(bytes('{"id":0}'), "id")), "0"); + /// @dev `0` is a JSON integer in its one spelling, so the reader returns + /// it; the `userId` grammar is what refuses it. + function test_readsZeroWhichIsNoUserId() public view { + bytes memory zero = this.jsonInteger(bytes('{"id":0}'), "id"); + assertEq(string(zero), "0"); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64(zero)); } function test_refusesAQuotedIntegerForTheIntegerTemplate() public { @@ -132,6 +136,38 @@ contract CeremonyFieldsTest is Test { this.jsonInteger(bytes('{"id":"1"}'), "id"); } + // ─── The userId grammar (REQ-PLAT-06) ─────────────────────────── + + function test_acceptsANonzeroUint64UpToItsMaximum() public pure { + assertTrue(CeremonyFields.isCanonicalNonzeroUint64("1")); + assertTrue(CeremonyFields.isCanonicalNonzeroUint64("2244994945")); + assertTrue(CeremonyFields.isCanonicalNonzeroUint64("18446744073709551615")); + } + + function test_refusesAValuePastUint64() public pure { + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("18446744073709551616")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("99999999999999999999")); + // Twenty-one digits. + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("100000000000000000000")); + } + + function test_refusesZeroAndALeadingZero() public pure { + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("0")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("007")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("01844674407370955161")); + } + + function test_refusesAnythingButDigits() public pure { + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("abc")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("12a")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("-1")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("+1")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("1.5")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("1e3")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64(" 1")); + } + // ─── Form fields ──────────────────────────────────────────────── function test_readsAnXTokenRequestBody() public view { diff --git a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol index 93b492d4..bac4b6c3 100644 --- a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol @@ -622,6 +622,26 @@ contract GitHubPlatformVerifierTest is Test { this.run{value: quote}(s); } + /// @dev REQ-PLAT-06. Each is a JSON integer the reader returns, and none + /// is a nonzero `uint64`. + function test_rejectsAnIdOutsideUint64() public { + string[3] memory ids = ["0", "18446744073709551616", "100000000000000000000"]; + for (uint256 i = 0; i < ids.length; ++i) { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = + _identity(string.concat('{"login":"octocat","id":', ids[i], "}"), CeremonyProfile.AUTHORITY_GITHUB_API); + vm.expectRevert(abi.encodeWithSelector(TlsNotaryVerifierBase.NoncanonicalUserId.selector, bytes(ids[i]))); + this.run{value: quote}(s); + } + } + + function test_acceptsTheLargestId() public { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = + _identity('{"login":"octocat","id":18446744073709551615}', CeremonyProfile.AUTHORITY_GITHUB_API); + assertEq(this.run{value: quote}(s).userId, "18446744073709551615"); + } + // ─── The handle field is `login` ──────────────────────────────── function test_rejectsAResponseWithNoLogin() public { diff --git a/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol b/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol index abbb2374..981ba5e2 100644 --- a/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol +++ b/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol @@ -151,6 +151,10 @@ contract LiveHelpers { return CeremonyFields.isSerializerSafe(value); } + function isCanonicalNonzeroUint64(bytes memory value) external pure returns (bool) { + return CeremonyFields.isCanonicalNonzeroUint64(value); + } + function occurrences(bytes memory haystack, bytes memory needle) external pure returns (uint256) { return CeremonyFields.occurrences(haystack, needle); } @@ -218,6 +222,10 @@ contract RefHelpers { return RefCeremonyFields.isSerializerSafe(value); } + function isCanonicalNonzeroUint64(bytes memory value) external pure returns (bool) { + return RefCeremonyFields.isCanonicalNonzeroUint64(value); + } + function occurrences(bytes memory haystack, bytes memory needle) external pure returns (uint256) { return RefCeremonyAttestation._occurrences(haystack, needle); } @@ -731,8 +739,8 @@ library Gen { { bytes memory ws = oneOf(r, list("", " ", "\n ", "\t")); bytes memory idValue = integerId - ? (chance(r, 80) ? bytes("293919812") : oneOf(r, list("0", "007", "12 3"))) - : (chance(r, 80) ? bytes('"1051915704843333634"') : oneOf(r, list('""', '"7', "7"))); + ? (chance(r, 80) ? bytes("293919812") : oneOf(r, list("0", "007", "12 3", "18446744073709551616"))) + : (chance(r, 80) ? bytes('"1051915704843333634"') : oneOf(r, list('""', '"7', "7", '"abc"'))); bytes memory idMember = abi.encodePacked('"id"', ws, ":", ws, idValue); bytes memory handleMember = abi.encodePacked('"', handleField, '"', ws, ":", ws, '"', oneOf(r, list("alice", "Bob_1", "", "a b")), '"'); @@ -959,6 +967,16 @@ contract TranscriptEquivalenceTest is Test { _same(address(live), address(ref), abi.encodeCall(LiveHelpers.isSerializerSafe, (value))); } + /// forge-config: default.fuzz.runs = 2000 + function testFuzz_userIdGrammarMatchesReference(uint256 seed, bytes memory raw) public view { + Gen.Rng memory r = Gen.Rng(seed); + // Digit runs either side of twenty, and one-byte edits of `2^64 - 1`. + bytes memory value = r.chance(25) + ? raw + : r.chance(50) ? r.soup("0123456789", 22) : r.mutate("18446744073709551615", "0123456789a"); + _same(address(live), address(ref), abi.encodeCall(LiveHelpers.isCanonicalNonzeroUint64, (value))); + } + /// forge-config: default.fuzz.runs = 2000 function testFuzz_revealedJoinMatchesReference(uint256 seed) public view { Gen.Rng memory r = Gen.Rng(seed); diff --git a/solidity/contracts/ceremony/test/TranscriptReference.sol b/solidity/contracts/ceremony/test/TranscriptReference.sol index 3562465b..46d3f8f1 100644 --- a/solidity/contracts/ceremony/test/TranscriptReference.sol +++ b/solidity/contracts/ceremony/test/TranscriptReference.sol @@ -234,7 +234,8 @@ library RefCeremonyAttestation { } /// @notice The field readers of `CeremonyFields` as of commit 7a63c20, code -/// unchanged. +/// unchanged, and the REQ-PLAT-06 `userId` grammar, written as a digit +/// scan and a comparison with the decimal of `2^64 - 1`. library RefCeremonyFields { error AmbiguousField(string name); error FieldNotFound(string name); @@ -449,6 +450,20 @@ library RefCeremonyFields { || c == "_" || c == "-"; } + function isCanonicalNonzeroUint64(bytes memory value) internal pure returns (bool) { + bytes memory max = "18446744073709551615"; + if (value.length == 0 || value.length > max.length || value[0] == "0") return false; + for (uint256 i = 0; i < value.length; ++i) { + if (value[i] < "0" || value[i] > "9") return false; + } + if (value.length < max.length) return true; + // Equal lengths of digits order as their numbers do. + for (uint256 i = 0; i < max.length; ++i) { + if (value[i] != max[i]) return value[i] < max[i]; + } + return true; + } + function _matchesAt(bytes memory data, bytes memory needle, uint256 at) private pure returns (bool) { for (uint256 j = 0; j < needle.length; ++j) { if (data[at + j] != needle[j]) return false; @@ -459,7 +474,8 @@ library RefCeremonyFields { /// @notice The transcript checks `TlsNotaryVerifierBase` ran on each session /// as of commit 7a63c20, code unchanged: everything between -/// authenticating an attestation and reading its time. +/// authenticating an attestation and reading its time. The identity +/// session also holds the id it reads to REQ-PLAT-06. /// @dev A profile's hooks are parameters here: `Profile` carries the /// constants `XPlatformVerifier` and `GitHubPlatformVerifier` return. library RefTranscript { @@ -467,6 +483,7 @@ library RefTranscript { error CodeVerifierMismatch(); error ClientIdentifierNotSerializerSafe(bytes found); error FieldNotUnique(string name, uint256 rangesMatching); + error NoncanonicalUserId(bytes found); error RequestLineNotAtOrigin(uint32 start); error WrongTokenRequestLayout(uint256 revealedRanges, uint256 commitments); error NoHeadBoundary(uint256 occurrences); @@ -579,11 +596,11 @@ library RefTranscript { RefCeremonyAttestation.requireExactCoverage(data.received, data.recvTranscriptLength); bytes memory joined = RefCeremonyAttestation.concatRevealed(data.received); - userId = string( - !profile.idIsInteger - ? _uniqueJsonString(data.received, joined, profile.idField) - : _uniqueJsonInteger(data.received, joined, profile.idField) - ); + bytes memory id = !profile.idIsInteger + ? _uniqueJsonString(data.received, joined, profile.idField) + : _uniqueJsonInteger(data.received, joined, profile.idField); + if (!RefCeremonyFields.isCanonicalNonzeroUint64(id)) revert NoncanonicalUserId(id); + userId = string(id); handle = string(_uniqueJsonString(data.received, joined, profile.handleField)); } diff --git a/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol index 7747a0f9..4b606d12 100644 --- a/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol @@ -1781,6 +1781,24 @@ contract XPlatformVerifierTest is Test { this.run{value: quote}(s); } + /// @dev REQ-PLAT-06. The string reader returns whatever lies between the + /// quotes, so the id grammar alone refuses each of these. + function test_rejectsAnIdOutsideTheGrammar() public { + string[6] memory ids = ["0", "007", "abc", "-1", "18446744073709551616", "100000000000000000000"]; + for (uint256 i = 0; i < ids.length; ++i) { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = _identityAttestation(ids[i], "alice", ""); + vm.expectRevert(abi.encodeWithSelector(TlsNotaryVerifierBase.NoncanonicalUserId.selector, bytes(ids[i]))); + this.run{value: quote}(s); + } + } + + function test_acceptsTheLargestId() public { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = _identityAttestation("18446744073709551615", "alice", ""); + assertEq(this.run{value: quote}(s).userId, "18446744073709551615"); + } + function test_theIdentityAttestationsOwnTimeIsNotEvidenceTime() public { TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); bytes memory a = s.identitySession.attestedData; diff --git a/solidity/snapshots/claim-verification.json b/solidity/snapshots/claim-verification.json index 87dd4011..eae91dd5 100644 --- a/solidity/snapshots/claim-verification.json +++ b/solidity/snapshots/claim-verification.json @@ -1,6 +1,6 @@ { "BearerLinkHonkVerifier.verify": "720457", - "GitHubPlatformVerifier.verify": "1146243", + "GitHubPlatformVerifier.verify": "1147664", "OidcGoogleHonkVerifier.verify": "755087", - "XPlatformVerifier.verify": "1094893" + "XPlatformVerifier.verify": "1097665" } \ No newline at end of file From 1b22217b03dee4ebf26adb56fae72068dabb1c10 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Mon, 5 Oct 2026 14:57:53 +0100 Subject: [PATCH 2/2] docs(ceremony): say every TLSNotary id must be a nonzero uint64 The base holds every profile's id to REQ-PLAT-06, so a new profile learns the rule from the _identityFields doc rather than a revert. The GitHub test that rejects 0 takes the X test's name: 0 is inside uint64 but outside the grammar. The reference copy's doc names its one change the way the file's other library does. Assisted-by: Claude Opus 5.5 Signed-off-by: xgreenx --- solidity/contracts/ceremony/TlsNotaryVerifierBase.sol | 3 ++- .../contracts/ceremony/test/GitHubPlatformVerifier.t.sol | 2 +- solidity/contracts/ceremony/test/TranscriptReference.sol | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol index 1a37f796..128f9a1e 100644 --- a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol +++ b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol @@ -176,7 +176,8 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa /// Declaring the field names and leaving the reading to the base makes /// all three unwritable rather than forbidden by a comment. A new /// profile supplies two strings and a shape; it never touches an - /// attestation. + /// attestation. Whatever the shape, the id must spell a nonzero + /// `uint64` in decimal (REQ-PLAT-06), as X and GitHub ids do. function _identityFields() internal pure diff --git a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol index bac4b6c3..d472fd8d 100644 --- a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol @@ -624,7 +624,7 @@ contract GitHubPlatformVerifierTest is Test { /// @dev REQ-PLAT-06. Each is a JSON integer the reader returns, and none /// is a nonzero `uint64`. - function test_rejectsAnIdOutsideUint64() public { + function test_rejectsAnIdOutsideTheGrammar() public { string[3] memory ids = ["0", "18446744073709551616", "100000000000000000000"]; for (uint256 i = 0; i < ids.length; ++i) { TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); diff --git a/solidity/contracts/ceremony/test/TranscriptReference.sol b/solidity/contracts/ceremony/test/TranscriptReference.sol index 46d3f8f1..9964bdfd 100644 --- a/solidity/contracts/ceremony/test/TranscriptReference.sol +++ b/solidity/contracts/ceremony/test/TranscriptReference.sol @@ -473,9 +473,9 @@ library RefCeremonyFields { } /// @notice The transcript checks `TlsNotaryVerifierBase` ran on each session -/// as of commit 7a63c20, code unchanged: everything between -/// authenticating an attestation and reading its time. The identity -/// session also holds the id it reads to REQ-PLAT-06. +/// as of commit 7a63c20: everything between authenticating an +/// attestation and reading its time. Code unchanged except the +/// REQ-PLAT-06 check on the id the identity session reads. /// @dev A profile's hooks are parameters here: `Profile` carries the /// constants `XPlatformVerifier` and `GitHubPlatformVerifier` return. library RefTranscript {