Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add cross-origin check #200

Merged
merged 4 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/batch/BatchCaller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) {
Expand Down
121 changes: 0 additions & 121 deletions src/validators/PasskeyValidator.sol

This file was deleted.

15 changes: 5 additions & 10 deletions src/validators/SessionKeyValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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 {
Expand Down
59 changes: 48 additions & 11 deletions src/validators/WebAuthValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}
8 changes: 4 additions & 4 deletions test/PasskeyModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand All @@ -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);
}

/**
Expand Down