Avoid revert caused by slice overflow in WebAuthn.verify

.changeset/spicy-seals-bake.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`WebAuthn`: Verification now returns `false` instead of reverting when client data contains an out-of-bounds `challengeIndex`.

contracts/utils/cryptography/WebAuthn.sol

@@ -4,6 +4,7 @@
 pragma solidity ^0.8.24;
 
 import {P256} from "./P256.sol";
+import {Math} from "../math/Math.sol";
 import {Base64} from "../Base64.sol";
 import {Bytes} from "../Bytes.sol";
 import {Strings} from "../Strings.sol";
@@ -156,7 +157,11 @@ library WebAuthn {
         // solhint-disable-next-line quotes
         string memory expectedChallenge = string.concat('"challenge":"', Base64.encodeURL(challenge), '"');
         string memory actualChallenge = string(
-            Bytes.slice(bytes(clientDataJSON), challengeIndex, challengeIndex + bytes(expectedChallenge).length)
+            Bytes.slice(
+                bytes(clientDataJSON),
+                challengeIndex,
+                Math.saturatingAdd(challengeIndex, bytes(expectedChallenge).length)
+            )
         );
 
         return Strings.equal(actualChallenge, expectedChallenge);

test/utils/cryptography/WebAuthn.t.sol

@@ -141,12 +141,54 @@ contract WebAuthnTest is Test {
         );
     }
 
+    /// forge-config: default.fuzz.runs = 512
+    function testVerifyIndexOutOfBounds(bytes memory challenge, uint256 seed) public view {
+        bytes memory authenticatorData = _encodeAuthenticatorData(WebAuthn.AUTH_DATA_FLAGS_UP);
+        string memory clientDataJSON = _encodeClientDataJSON(challenge);
+
+        assertFalse(
+            _runVerify(
+                seed,
+                challenge,
+                bytes(clientDataJSON).length, // end of the clientDataJSON, no more room slicing
+                1,
+                authenticatorData,
+                clientDataJSON,
+                false
+            )
+        );
+
+        assertFalse(
+            _runVerify(
+                seed,
+                challenge,
+                type(uint256).max, // way out of bound, causing an overflow when figuring out the end of the slice
+                1,
+                authenticatorData,
+                clientDataJSON,
+                false
+            )
+        );
+    }
+
     function _runVerify(
         uint256 seed,
         bytes memory challenge,
         bytes memory authenticatorData,
         string memory clientDataJSON,
         bool requireUV
+    ) private view returns (bool) {
+        return _runVerify(seed, challenge, 23, 1, authenticatorData, clientDataJSON, requireUV);
+    }
+
+    function _runVerify(
+        uint256 seed,
+        bytes memory challenge,
+        uint256 challengeIndex,
+        uint256 typeIndex,
+        bytes memory authenticatorData,
+        string memory clientDataJSON,
+        bool requireUV
     ) private view returns (bool) {
         // Generate private key and get public key
         uint256 privateKey = bound(seed, 1, P256.N - 1);
@@ -163,8 +205,8 @@ contract WebAuthnTest is Test {
                 WebAuthn.WebAuthnAuth({
                     authenticatorData: authenticatorData,
                     clientDataJSON: clientDataJSON,
-                    challengeIndex: 23, // Position of challenge in clientDataJSON
-                    typeIndex: 1, // Position of type in clientDataJSON
+                    challengeIndex: challengeIndex, // Position of challenge in clientDataJSON
+                    typeIndex: typeIndex, // Position of type in clientDataJSON
                     r: r,
                     s: bytes32(Math.min(uint256(s), P256.N - uint256(s)))
                 }),