check requestId correctness

iden3 · Jan 16, 2025 · 296bb68 · 296bb68
1 parent d510368
commit 296bb68
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 6 deletions.
diff --git a/contracts/validators/AuthV2Validator_forAuth.sol b/contracts/validators/AuthV2Validator_forAuth.sol
@@ -134,7 +134,7 @@ contract AuthV2Validator_forAuth is Ownable2StepUpgradeable, IAuthValidator, ERC
 
         PubSignals memory pubSignals = parsePubSignals(inputs);
         _checkGistRoot(pubSignals.userID, pubSignals.gistRoot, state);
-        _checkChallenge(pubSignals.challenge, expectedNonce); // expectedNonce
+        _checkChallenge(pubSignals.challenge, expectedNonce);
         _verifyZKP(inputs, a, b, c);
         return pubSignals.userID;
     }

diff --git a/contracts/verifiers/Verifier.sol b/contracts/verifiers/Verifier.sol
@@ -28,6 +28,8 @@ error MetadataNotSupportedYet();
 error GroupMustHaveAtLeastTwoRequests(uint256 groupID);
 error NullifierSessionIDAlreadyExists(uint256 nullifierSessionID);
 error VerifierIDIsNotValid(uint256 requestVerifierID, uint256 expectedVerifierID);
+error RequestIdNotValid();
+error RequestIdUsesReservedBytes();
 
 abstract contract Verifier is IVerifier, ContextUpgradeable {
     /// @dev Key to retrieve the linkID from the proof storage
@@ -285,6 +287,8 @@ abstract contract Verifier is IVerifier, ContextUpgradeable {
 
         // 2. Set requests checking groups and nullifierSessionID uniqueness
         for (uint256 i = 0; i < requests.length; i++) {
+            _checkRequestIdCorrectness(requests[i].requestId);
+
             _checkNullifierSessionIdUniqueness(requests[i]);
             _checkVerifierID(requests[i]);
 
@@ -316,6 +320,24 @@ abstract contract Verifier is IVerifier, ContextUpgradeable {
         }
     }
 
+    function _getRequestType(uint256 requestId) internal pure returns (uint8) {
+        // 0x0000000000000000 - prefix for old uint64 requests
+        // 0x0000000000000001 - prefix for keccak256 cut to fit in the remaining 192 bits
+        return uint8(requestId >> 248);
+    }
+
+    function _checkRequestIdCorrectness(uint256 requestId) internal pure {
+        // 1. Check prefix
+        uint8 requestType = _getRequestType(requestId);
+        if (requestType >= 2) {
+            revert RequestIdNotValid();
+        }
+        // 2. Check reserved bytes
+        if (((requestId << 8) >> 200) > 0) {
+            revert RequestIdUsesReservedBytes();
+        }
+    }
+
     function _checkNullifierSessionIdUniqueness(IVerifier.Request calldata request) internal {
         VerifierStorage storage s = _getVerifierStorage();
         uint256 nullifierSessionID = request

diff --git a/test/validators/authv2/index.ts b/test/validators/authv2/index.ts
@@ -27,7 +27,6 @@ const testCases: any[] = [
   {
     name: "Validation of Gist root not found",
     sender: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266",
-    challenge: "0x00",
     stateTransitions: [
       require("../common-data/issuer_from_genesis_state_to_first_transition_v3.json"),
     ],

diff --git a/test/verifier/universal-verifier-submit-V2.test.ts b/test/verifier/universal-verifier-submit-V2.test.ts
@@ -197,9 +197,9 @@ describe("Universal Verifier submitResponse SigV2 validators", function () {
     expect(status.validatorVersion).to.be.equal("1.0.0-stub");
     expect(status.timestamp).to.be.equal(txResTimestamp);
 
-    await expect(verifier.getRequestStatus(signerAddress, nonExistingRequestId)).to.be.rejectedWith(
-      `RequestIdNotFound(${nonExistingRequestId})`,
-    );
+    await expect(verifier.getRequestStatus(signerAddress, nonExistingRequestId))
+      .to.be.revertedWithCustomError(verifier, "RequestIdNotFound")
+      .withArgs(nonExistingRequestId);
 
     const requestIdsMulti = requestIds.slice(1, 3);
     const txMulti = await verifier.submitResponse(

diff --git a/test/verifier/universal-verifier.v3.test.ts b/test/verifier/universal-verifier.v3.test.ts
@@ -559,7 +559,9 @@ describe("Universal Verifier V3 validator", function () {
           params: params2,
         },
       ]),
-    ).to.be.rejectedWith(`NullifierSessionIDAlreadyExists(${query2.nullifierSessionID})`);
+    )
+      .to.be.revertedWithCustomError(verifier, "NullifierSessionIDAlreadyExists")
+      .withArgs(query2.nullifierSessionID);
   });
 
   it("Test set request fails with VerifierIDIsNotValid", async () => {

diff --git a/test/verifier/verifer.test.ts b/test/verifier/verifer.test.ts
@@ -68,6 +68,23 @@ describe("Verifer tests", function () {
         .withArgs(1);
     });
 
+    it("setRequests: requestId should be valid and not using reserved bytes", async function () {
+      await validator.stub_setRequestParams([request.params], [paramsFromValidator]);
+
+      request.requestId = BigInt(2 ** 256) - BigInt(1);
+
+      await expect(verifier.setRequests([request])).to.be.revertedWithCustomError(
+        verifier,
+        "RequestIdNotValid",
+      );
+
+      request.requestId = BigInt(2 ** 248) + BigInt(2 ** 247);
+      await expect(verifier.setRequests([request])).to.be.revertedWithCustomError(
+        verifier,
+        "RequestIdUsesReservedBytes",
+      );
+    });
+
     it("submitResponse: not repeated responseFields from validator", async function () {
       await verifier.setRequests([request]);
       await validator.stub_setVerifyResults([