Reject non-canonical P-384 public keys

Co-authored-by: OpenCode <opencode-noreply@coinbase.com>

Author: leopoldjoy

CHANGELOG.md

@@ -4,6 +4,11 @@ All notable changes to this project are documented here. The format is based on
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project aims to follow
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+- Reject non-canonical P-384 public key coordinates greater than or equal to the field prime `p`.
+
 ## [2.0.0-rc.1] - 2026-06-09
 
 First release candidate of the **hinted P-384** rework. This is a major, breaking change motivated

docs/hinted-p384-nitro-attestation.md

@@ -291,12 +291,14 @@ Both moduli (`p` and `n`) are prime, so any nonzero element has a *unique* inver
   most cause a revert (wasted gas), never a false accept. (A non-canonical hint such
   as `inv + m` also passes the check and is harmless, since every use reduces mod `m`.)
 
-The equivalence anchor for this argument is that with hints **disabled** the code is
-identical to the upstream verifier; the hinted branches only *substitute a
-pre-verified value* for the same inverse the original computes.
+The equivalence anchor for this argument is that, aside from rejecting non-canonical
+public key coordinates, the hints-disabled code follows the upstream verifier; the
+hinted branches only *substitute a pre-verified value* for the same inverse the original
+computes.
 
 ### What a failed verification does
-- With hints **disabled**, behavior is identical to the original verifier.
+- With hints **disabled**, behavior is identical to the original verifier except that
+  non-canonical public key coordinates are rejected.
 - A *well-formed but invalid* signature consumes its full hint stream and the verify
   returns `false` normally.
 - A signature that fails an **early** guard (scalar bounds or the on-curve check)
@@ -471,17 +473,19 @@ The off-chain↔on-chain hint equivalence is checked under an FFI test (it shell
 the generator and compares streams byte-for-byte), so the generator and the contract
 can never silently diverge.
 
-**For auditors.** The entire trust delta versus the upstream library is the two
-`if (hintsEnabled)` branches shown in the appendix; the soundness argument is in §4
-(*Why this is sound*). Reviewers should confirm:
+**For auditors.** The main trust delta versus the upstream library is the two
+`if (hintsEnabled)` branches shown in the appendix; this repo also tightens public-key
+coordinate bounds to reject non-canonical field encodings. The hinting soundness
+argument is in §4 (*Why this is sound*). Reviewers should confirm:
 
 1. every supplied inverse is constrained by `b · inv ≡ 1` in the correct modulus
    **before** it is used;
 2. point inverses use the field prime `p` and scalar inverses use the group order `n`,
    with no crossover;
 3. the underflow and surplus guards together force the hint count to match exactly
    (no truncated or leftover hints);
-4. the hints-disabled path is byte-identical to the upstream verifier; and
+4. the hints-disabled path differs from upstream only by the stricter public-key
+   coordinate bounds; and
 5. the curve parameters (`p`, `n`, `G`, `a`, `b`, low-`s` bound) are correct.
 
 ## 10. Caveats and notes
@@ -494,9 +498,10 @@ can never silently diverge.
   `x_R mod n == r`, exactly as the upstream library, including its handling of the
   negligible (~2⁻¹⁹⁰) case where the recovered x-coordinate lies in `[n, p)`. Hinting
   does not alter this.
-- **Audit boundary.** Only the two inversion branches are new cryptographic code; the
-  rest of the verifier is the upstream library unchanged, and the certificate parser,
-  cache, and CBOR/COSE handling are the pre-existing validator logic.
+- **Audit boundary.** The hinted inversion branches are the primary new cryptographic
+  code; this repo also tightens public-key coordinate bounds versus upstream. The
+  certificate parser, cache, and CBOR/COSE handling are the pre-existing validator
+  logic.
 - **The generator is liveness-critical, not trust-critical.** A bug in the off-chain
   hint generator can only cause a revert (every value is re-checked on-chain), never a
   false accept — but correct hints are required to verify at all, so the generator
@@ -551,12 +556,11 @@ The hinted verifier is the upstream `ECDSA384` verifier from
 `dl-solarity/solidity-lib`, vendored into this repo at `src/vendor/ECDSA384.sol` (see
 `src/vendor/README.md` for provenance and the exact upstream diff in
 `src/vendor/ECDSA384.hinted.patch`) with **one operation — modular inversion — made
-hint-aware in two places**. Everything else (the Strauss–Shamir ladder, precompute
-table, on-curve check, scalar bounds, final `x_R == r`) is unchanged. When hints are
-disabled the code follows the original path, so the unhinted path is the equivalence
-anchor.
+hint-aware in two places**, plus stricter public-key coordinate bounds. Everything else
+(the Strauss–Shamir ladder, precompute table, scalar bounds, final `x_R == r`) is
+unchanged.
 
-The entire trust delta is these two `if (hintsEnabled)` branches.
+The hinting trust delta is these two `if (hintsEnabled)` branches.
 
 **Point arithmetic — inverses mod `p`** (`moddivAssign`):
 
@@ -613,12 +617,15 @@ no-truncation guard:
 +    }
 ```
 
+The public-key on-curve guard is also tightened to reject non-canonical coordinates
+`>= p`, not only coordinates equal to `p`.
+
 The remaining changes are **plumbing**, not logic, and none can affect the
 accept/reject decision:
 
 - a few words of scratch memory holding the hint stream pointer, length, cursor, and
   an enabled flag (`initCall` / `initCallWithHints`);
-- the `verify` entrypoint split into `verify` (no hints, identical to the original)
+- the `verify` entrypoint split into `verify` (no hints)
   and `verifyWithHints`, which adds the surplus guard
   `require(consumed == length, "unused inverse hints")`.
 

src/vendor/ECDSA384.hinted.patch

@@ -75,6 +75,16 @@ index 9e43756..053803b 100644
          }
      }
 
+@@ -168,7 +202,10 @@ library ECDSA384 {
+     ) private view returns (bool) {
+         unchecked {
+-            if (U384.eqInteger(x, 0) || U384.eq(x, p) || U384.eqInteger(y, 0) || U384.eq(y, p)) {
++            if (
++                U384.eqInteger(x, 0) || U384.cmp(x, p) >= 0 || U384.eqInteger(y, 0) || U384.cmp(y, p) >= 0
++            ) {
+                 return false;
+             }
+
 @@ -486,11 +520,14 @@ library ECDSA384 {
  library U384 {
      uint256 private constant SHORT_ALLOCATION = 64;

src/vendor/ECDSA384.sol

@@ -2,9 +2,10 @@
 //
 // Vendored from Solarity solidity-lib: https://github.com/dl-solarity/solidity-lib
 //   Base (upstream, unmodified): commit b947757194de6436062c2d68118c0352be84ac4be
-//   Local modification: "Add hinted P384 inverse verification" (verifyWithHints /
-//   verifyWithHintsConsumed and the U384 hint-consumption paths). The exact upstream
-//   diff is committed alongside this file as ECDSA384.hinted.patch for review.
+//   Local modifications: "Add hinted P384 inverse verification" (verifyWithHints /
+//   verifyWithHintsConsumed and the U384 hint-consumption paths), plus strict public
+//   key coordinate bounds. The exact upstream diff is committed alongside this file
+//   as ECDSA384.hinted.patch for review.
 // Copyright (c) 2023 Solarity. Originally licensed MIT (see header above).
 pragma solidity ^0.8.4;
 
@@ -201,7 +202,9 @@ library ECDSA384 {
         uint256 y
     ) private view returns (bool) {
         unchecked {
-            if (U384.eqInteger(x, 0) || U384.eq(x, p) || U384.eqInteger(y, 0) || U384.eq(y, p)) {
+            if (
+                U384.eqInteger(x, 0) || U384.cmp(x, p) >= 0 || U384.eqInteger(y, 0) || U384.cmp(y, p) >= 0
+            ) {
                 return false;
             }
 

src/vendor/README.md

@@ -18,13 +18,17 @@ the code that is deployed — with no dependency on an external submodule or for
 The only imports either file makes are between these two vendored files; nothing else
 from `solidity-lib` is used by this repo.
 
-## The one local modification (audit focus)
+## Local modifications (audit focus)
 
-`ECDSA384.sol` carries a single functional change on top of the base commit:
-**"Add hinted P384 inverse verification"** — adding `verifyWithHints` /
-`verifyWithHintsConsumed` and the hint-consumption paths in `U384`
-(`initCallWithHints`, `_nextInverseHint`, the hinted branches of `moddivAssign` /
-`modinv`).
+`ECDSA384.sol` carries two functional changes on top of the base commit:
+
+- **"Add hinted P384 inverse verification"** — adding `verifyWithHints` /
+  `verifyWithHintsConsumed` and the hint-consumption paths in `U384`
+  (`initCallWithHints`, `_nextInverseHint`, the hinted branches of `moddivAssign` /
+  `modinv`).
+- **Strict public key coordinate bounds** — `_isOnCurve` rejects coordinates `>= p`,
+  not only `== p`, so non-canonical 48-byte field encodings cannot pass the curve
+  equation modulo `p`.
 
 The exact upstream diff is committed next to the file as
 [`ECDSA384.hinted.patch`](./ECDSA384.hinted.patch) so reviewers can see precisely

test/helpers/ECDSA384HintCollector.sol

@@ -186,8 +186,8 @@ library ECDSA384HintCollectorLib {
     function _isOnCurve(uint256 call, uint256 p, uint256 a, uint256 b, uint256 x, uint256 y) private returns (bool) {
         unchecked {
             if (
-                U384HintCollector.eqInteger(x, 0) || U384HintCollector.eq(x, p) || U384HintCollector.eqInteger(y, 0)
-                    || U384HintCollector.eq(y, p)
+                U384HintCollector.eqInteger(x, 0) || U384HintCollector.cmp(x, p) >= 0
+                    || U384HintCollector.eqInteger(y, 0) || U384HintCollector.cmp(y, p) >= 0
             ) {
                 return false;
             }

test/hinted/HintedNitroAttestation.t.sol

@@ -273,6 +273,24 @@ contract HintedNitroAttestationTest is Test {
         assertFalse(p384Verifier.verifyP384SignatureWithHints(hash, abi.encodePacked(one48, max48), pubKey, ""));
     }
 
+    function test_P384VerifierRejectsNonCanonicalPubKeyCoordinate() public view {
+        bytes memory hash = new bytes(48);
+        bytes memory one48 = new bytes(48);
+        one48[47] = 0x01;
+
+        // x = p + 2 fits in 48 bytes and reduces to a valid P-384 x-coordinate modulo p.
+        bytes memory nonCanonicalX =
+            hex"fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffeffffffff000000000000000100000001";
+        bytes memory y =
+            hex"8cdeadbbd04911a3c1931e26df3fa6439dca9c7eb286fbd46fc319f0e2bb780232baf57825fc0c1912ada2fefe84024c";
+
+        assertFalse(
+            p384Verifier.verifyP384SignatureWithHints(
+                hash, abi.encodePacked(one48, one48), abi.encodePacked(nonCanonicalX, y), ""
+            )
+        );
+    }
+
     function test_OffchainWitnessGeneratorMatchesSolidityCollector() public {
         if (!vm.envOr("NITRO_RUN_FFI", false)) {
             console.log("==== OFFCHAIN WITNESS GENERATOR ====");

tools/p384_hints.js

@@ -162,7 +162,7 @@ function collectAttestationHintBytes(attestation, leafPubKey) {
 }
 
 function isOnCurve(x, y) {
-  if (x === 0n || x === P || y === 0n || y === P) {
+  if (x === 0n || x >= P || y === 0n || y >= P) {
     return false;
   }