Prevent webauthn type check out-of-gas

[james-toussaint]

Fixes L-08.

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9955dcf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

This PR modifies the WebAuthn signature verification logic in WebAuthn.sol to add explicit bounds checking. An out-of-bounds read protection is introduced in the private _validateExpectedTypeHash function by validating that clientDataJSON has sufficient length (typeIndex + 21 bytes) before accessing the type field. The inline assembly logic was refactored to remove a redundant length comparison condition while retaining the substring equality check. A corresponding test case verifies that verification fails when an out-of-bounds typeIndex value is supplied.

Possibly related PRs

• Avoid revert caused by slice overflow in WebAuthn.verify #6329: Adds bounds checks to WebAuthn.sol to prevent out-of-range reads during WebAuthn.verify operations, directly addressing the same safety concern as this PR.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Prevent webauthn type check out-of-gas' directly addresses the main change: adding a bounds check to prevent out-of-bounds reads during WebAuthn type validation.
Description check	✅ Passed	The description references 'Fixes L-08' and mentions adding tests and code changes to prevent an out-of-gas condition in WebAuthn type validation, which aligns with the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

contracts/utils/cryptography/WebAuthn.sol (1)

136-136: Potential integer overflow when typeIndex is extremely large.

If typeIndex > type(uint256).max - 21, the addition typeIndex + 21 will overflow and revert in Solidity 0.8+. This causes a revert instead of gracefully returning false. For consistency with _validateChallenge (which uses Math.saturatingAdd), consider using saturating arithmetic here.

♻️ Suggested fix using saturating arithmetic

-        if (bytes(clientDataJSON).length < typeIndex + 21) return false;
+        if (bytes(clientDataJSON).length < Math.saturatingAdd(typeIndex, 21)) return false;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/utils/cryptography/WebAuthn.sol` at line 136, The length check
using "if (bytes(clientDataJSON).length < typeIndex + 21) return false;" can
overflow when typeIndex is near uint256 max; update this to use saturating
addition (the same approach as _validateChallenge) so the comparison cannot
overflow—e.g., compute a saturating sum of typeIndex and 21 via
Math.saturatingAdd (or equivalent helper) and compare
bytes(clientDataJSON).length against that saturating sum; ensure you reference
typeIndex, clientDataJSON, and Math.saturatingAdd/_validateChallenge when making
the change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@contracts/utils/cryptography/WebAuthn.sol`:
- Line 136: The length check using "if (bytes(clientDataJSON).length < typeIndex
+ 21) return false;" can overflow when typeIndex is near uint256 max; update
this to use saturating addition (the same approach as _validateChallenge) so the
comparison cannot overflow—e.g., compute a saturating sum of typeIndex and 21
via Math.saturatingAdd (or equivalent helper) and compare
bytes(clientDataJSON).length against that saturating sum; ensure you reference
typeIndex, clientDataJSON, and Math.saturatingAdd/_validateChallenge when making
the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d1193455-8237-469c-b968-7a96179a92aa

📥 Commits

Reviewing files that changed from the base of the PR and between 45f032d and 18fad5a.

📒 Files selected for processing (2)

• contracts/utils/cryptography/WebAuthn.sol
• test/utils/cryptography/WebAuthn.t.sol

[Amxx]

The test that is added here doesn't trigger the out of gas when running with the old version of the library. So its not properly validating the behavior we want to fix because it doesn't trigger it.

Note: we already have tests with high challengeIndex in testVerifyIndexOutOfBounds.