fix: add cross-origin check (#200)

* fix: add cross-origin check We don't want to allow cross-origin requests * fix: small changes (#205) * fix: small changes * fix: restore tests and fix crossOrigin bug * fix: typechain --------- Co-authored-by: Lyova Potyomkin <[email protected]> --------- Co-authored-by: Vlad Bochok <[email protected]> Co-authored-by: Lyova Potyomkin <[email protected]>
matter-labs · Nov 26, 2024 · bfdc6fb · bfdc6fb
1 parent eeff83b
commit bfdc6fb
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 152 deletions.
diff --git a/src/batch/BatchCaller.sol b/src/batch/BatchCaller.sol
@@ -5,6 +5,7 @@ import { SystemContractsCaller } from "@matterlabs/zksync-contracts/l2/system-co
 import { EfficientCall } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/EfficientCall.sol";
 import { DEPLOYER_SYSTEM_CONTRACT } from "@matterlabs/zksync-contracts/l2/system-contracts/Constants.sol";
 import { Errors } from "../libraries/Errors.sol";
+import { SelfAuth } from "../auth/SelfAuth.sol";
 
 /// @dev Represents an external call data.
 /// @param target The address to which the call will be made.
@@ -23,16 +24,12 @@ struct Call {
 /// @custom:security-contact [email protected]
 /// @notice Make multiple calls from Account in a single transaction.
 /// @notice The implementation is inspired by Clave wallet.
-abstract contract BatchCaller {
+abstract contract BatchCaller is SelfAuth {
   /// @notice Make multiple calls, ensure success if required.
   /// @dev The total Ether sent across all calls must be equal to `msg.value` to maintain the invariant
   /// that `msg.value` + `tx.fee` is the maximum amount of Ether that can be spent on the transaction.
   /// @param _calls Array of Call structs, each representing an individual external call to be made.
-  function batchCall(Call[] calldata _calls) external payable {
-    if (msg.sender != address(this)) {
-      revert Errors.NOT_FROM_SELF();
-    }
-
+  function batchCall(Call[] calldata _calls) external payable onlySelf {
     uint256 totalValue;
     uint256 len = _calls.length;
     for (uint256 i = 0; i < len; ++i) {

diff --git a/src/validators/PasskeyValidator.sol b/src/validators/PasskeyValidator.sol
diff --git a/src/validators/SessionKeyValidator.sol b/src/validators/SessionKeyValidator.sol
@@ -82,20 +82,16 @@ contract SessionKeyValidator is IValidationHook, IModuleValidator, IModule {
 
   function disable() external {
     if (_isInitialized(msg.sender)) {
-      _uninstall();
+      // Here we have to revoke all keys, so that if the module
+      // is installed again later, there will be no active sessions from the past.
+      // Problem: if there are too many keys, this will run out of gas.
+      // Solution: before uninstalling, require that all keys are revoked manually.
+      require(sessionCounter[msg.sender] == 0, "Revoke all keys first");
       IValidatorManager(msg.sender).removeModuleValidator(address(this));
       IHookManager(msg.sender).removeHook(address(this), true);
     }
   }
 
-  function _uninstall() internal {
-    // Here we have to revoke all keys, so that if the module
-    // is installed again later, there will be no active sessions from the past.
-    // Problem: if there are too many keys, this will run out of gas.
-    // Solution: before uninstalling, require that all keys are revoked manually.
-    require(sessionCounter[msg.sender] == 0, "Revoke all keys first");
-  }
-
   function supportsInterface(bytes4 interfaceId) external pure override returns (bool) {
     return
       interfaceId != 0xffffffff &&
@@ -130,7 +126,6 @@ contract SessionKeyValidator is IValidationHook, IModuleValidator, IModule {
 
   function _isInitialized(address smartAccount) internal view returns (bool) {
     return IHookManager(smartAccount).isHook(address(this));
-    // && IValidatorManager(smartAccount).isModuleValidator(address(this));
   }
 
   function validationHook(bytes32 signedHash, Transaction calldata transaction, bytes calldata hookData) external {

diff --git a/src/validators/WebAuthValidator.sol b/src/validators/WebAuthValidator.sol
@@ -2,13 +2,21 @@
 pragma solidity ^0.8.24;
 
 import { IModuleValidator } from "../interfaces/IModuleValidator.sol";
-import "./PasskeyValidator.sol";
+import { VerifierCaller } from "../helpers/VerifierCaller.sol";
+import { JsmnSolLib } from "../libraries/JsmnSolLib.sol";
+import { Strings } from "../helpers/EIP712.sol";
+import { Base64 } from "../helpers/Base64.sol";
+import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
+
+/// @title AAFactory
+/// @author Matter Labs
+/// @custom:security-contact [email protected]
+/// @dev This contract allows secure user authentication using WebAuthn public keys.
+contract WebAuthValidator is VerifierCaller, IModuleValidator {
+  address constant P256_VERIFIER = address(0x100);
+  bytes1 constant AUTH_DATA_MASK = 0x05;
+  bytes32 constant lowSmax = 0x7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8;
 
-/**
- * @title validator contract for passkey r1 signatures
- * @author https://getclave.io
- */
-contract WebAuthValidator is PasskeyValidator, IModuleValidator {
   // The layout is weird due to EIP-7562 storage read restrictions for validation phase.
   mapping(string originDomain => mapping(address accountAddress => bytes32)) public lowerKeyHalf;
   mapping(string originDomain => mapping(address accountAddress => bytes32)) public upperKeyHalf;
@@ -57,6 +65,7 @@ contract WebAuthValidator is PasskeyValidator, IModuleValidator {
     bool validChallenge = false;
     bool validType = false;
     bool validOrigin = false;
+    bool validCrossOrigin = true;
     for (uint256 index = 1; index < actualNum; index++) {
       JsmnSolLib.Token memory t = tokens[index];
       if (t.jsmnType == JsmnSolLib.JsmnType.STRING) {
@@ -97,21 +106,49 @@ contract WebAuthValidator is PasskeyValidator, IModuleValidator {
 
           // This really only validates the origin is set
           validOrigin = pubKey[0] != 0 && pubKey[1] != 0;
+        } else if (Strings.equal(keyOrValue, "crossOrigin")) {
+          JsmnSolLib.Token memory nextT = tokens[index + 1];
+          string memory crossOriginValue = JsmnSolLib.getBytes(clientDataJSON, nextT.start, nextT.end);
+          // this should only be set once, otherwise this is an error
+          if (!validCrossOrigin) {
+            return false;
+          }
+          validCrossOrigin = Strings.equal("false", crossOriginValue);
         }
-        // TODO: check 'cross-origin' keys as part of signature
       }
     }
 
-    if (!validChallenge || !validType) {
+    if (!validChallenge || !validType || !validOrigin || !validCrossOrigin) {
       return false;
     }
 
     bytes32 message = _createMessage(authenticatorData, bytes(clientDataJSON));
     valid = callVerifier(P256_VERIFIER, message, rs, pubKey);
   }
 
-  /// @inheritdoc IERC165
-  function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
-    return super.supportsInterface(interfaceId) || interfaceId == type(IModuleValidator).interfaceId;
+  function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
+    return interfaceId == type(IERC165).interfaceId || interfaceId == type(IModuleValidator).interfaceId;
+  }
+
+  function _createMessage(
+    bytes memory authenticatorData,
+    bytes memory clientData
+  ) internal pure returns (bytes32 message) {
+    bytes32 clientDataHash = sha256(clientData);
+    message = sha256(bytes.concat(authenticatorData, clientDataHash));
+  }
+
+  function _decodeFatSignature(
+    bytes memory fatSignature
+  ) internal pure returns (bytes memory authenticatorData, string memory clientDataSuffix, bytes32[2] memory rs) {
+    (authenticatorData, clientDataSuffix, rs) = abi.decode(fatSignature, (bytes, string, bytes32[2]));
+  }
+
+  function rawVerify(
+    bytes32 message,
+    bytes32[2] calldata rs,
+    bytes32[2] calldata pubKey
+  ) external view returns (bool valid) {
+    valid = callVerifier(P256_VERIFIER, message, rs, pubKey);
   }
 }
diff --git a/test/PasskeyModule.ts b/test/PasskeyModule.ts
@@ -8,7 +8,7 @@ import { assert } from "chai";
 import * as hre from "hardhat";
 import { Wallet } from "zksync-ethers";
 
-import { PasskeyValidator, PasskeyValidator__factory } from "../typechain-types";
+import { WebAuthValidator, WebAuthValidator__factory } from "../typechain-types";
 import { getWallet, LOCAL_RICH_WALLETS, RecordedResponse } from "./utils";
 
 /**
@@ -29,14 +29,14 @@ export function toBuffer(
 
 async function deployValidator(
   wallet: Wallet,
-): Promise<PasskeyValidator> {
+): Promise<WebAuthValidator> {
   const deployer: Deployer = new Deployer(hre, wallet);
   const passkeyValidatorArtifact = await deployer.loadArtifact(
-    "PasskeyValidator",
+    "WebAuthValidator",
   );
 
   const validator = await deployer.deploy(passkeyValidatorArtifact, []);
-  return PasskeyValidator__factory.connect(await validator.getAddress(), wallet);
+  return WebAuthValidator__factory.connect(await validator.getAddress(), wallet);
 }
 
 /**