diff --git a/src/SsoAccount.sol b/src/SsoAccount.sol index 20e793c9..59d9c67b 100644 --- a/src/SsoAccount.sol +++ b/src/SsoAccount.sol @@ -39,16 +39,16 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback /// @notice Initializer function that sets account initial configuration. Expected to be used in the proxy. /// @dev Sets passkey and passkey validator within account storage - /// @param _initialValidators An array of module validator addresses and initial validation keys + /// @param initialValidators An array of module validator addresses and initial validation keys /// in an ABI encoded format of `abi.encode(validatorAddr,validationKey))`. - /// @param _initialK1Owners An array of addresses with full control over the account. - function initialize(bytes[] calldata _initialValidators, address[] calldata _initialK1Owners) external initializer { - for (uint256 i = 0; i < _initialValidators.length; ++i) { - (address validatorAddr, bytes memory validationKey) = abi.decode(_initialValidators[i], (address, bytes)); + /// @param initialK1Owners An array of addresses with full control over the account. + function initialize(bytes[] calldata initialValidators, address[] calldata initialK1Owners) external initializer { + for (uint256 i = 0; i < initialValidators.length; ++i) { + (address validatorAddr, bytes memory validationKey) = abi.decode(initialValidators[i], (address, bytes)); _addModuleValidator(validatorAddr, validationKey); } - for (uint256 i = 0; i < _initialK1Owners.length; ++i) { - _k1AddOwner(_initialK1Owners[i]); + for (uint256 i = 0; i < initialK1Owners.length; ++i) { + _k1AddOwner(initialK1Owners[i]); } } diff --git a/src/handlers/ERC1271Handler.sol b/src/handlers/ERC1271Handler.sol index a8b4593f..a89d0e79 100644 --- a/src/handlers/ERC1271Handler.sol +++ b/src/handlers/ERC1271Handler.sol @@ -18,26 +18,23 @@ abstract contract ERC1271Handler is IERC1271Upgradeable, EIP712("Sso1271", "1.0. bytes32 signedHash; } - bytes32 constant _SSO_MESSAGE_TYPEHASH = keccak256("SsoMessage(bytes32 signedHash)"); + bytes32 private constant _SSO_MESSAGE_TYPEHASH = keccak256("SsoMessage(bytes32 signedHash)"); bytes4 private constant _ERC1271_MAGIC = 0x1626ba7e; /** * @dev Should return whether the signature provided is valid for the provided data - * @param signedHash bytes32 - Hash of the data that is signed - * @param signatureAndValidator bytes calldata - Validator address concatenated to signature + * @param hash bytes32 - Hash of the data that is signed + * @param signature bytes calldata - Validator address concatenated to signature * @return magicValue bytes4 - Magic value if the signature is valid, 0 otherwise */ - function isValidSignature( - bytes32 signedHash, - bytes memory signatureAndValidator - ) public view override returns (bytes4 magicValue) { - (bytes memory signature, address validator) = SignatureDecoder.decodeSignatureNoHookData(signatureAndValidator); + function isValidSignature(bytes32 hash, bytes memory signature) external view override returns (bytes4 magicValue) { + (bytes memory decodedSignature, address validator) = SignatureDecoder.decodeSignatureNoHookData(signature); - bytes32 eip712Hash = _hashTypedDataV4(_ssoMessageHash(SsoMessage(signedHash))); + bytes32 eip712Hash = _hashTypedDataV4(_ssoMessageHash(SsoMessage(hash))); bool isValid = _isModuleValidator(validator) && - IModuleValidator(validator).validateSignature(eip712Hash, signature); + IModuleValidator(validator).validateSignature(eip712Hash, decodedSignature); magicValue = isValid ? _ERC1271_MAGIC : bytes4(0); } @@ -59,7 +56,7 @@ abstract contract ERC1271Handler is IERC1271Upgradeable, EIP712("Sso1271", "1.0. return _SSO_MESSAGE_TYPEHASH; } - function _ssoMessageHash(SsoMessage memory ssoMessage) internal pure returns (bytes32) { + function _ssoMessageHash(SsoMessage memory ssoMessage) private pure returns (bytes32) { return keccak256(abi.encode(_SSO_MESSAGE_TYPEHASH, ssoMessage.signedHash)); } } diff --git a/src/handlers/ValidationHandler.sol b/src/handlers/ValidationHandler.sol index 276a7245..2abf4cf0 100644 --- a/src/handlers/ValidationHandler.sol +++ b/src/handlers/ValidationHandler.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.24; import { Transaction } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/TransactionHelper.sol"; import { SignatureDecoder } from "../libraries/SignatureDecoder.sol"; -import { BytesLinkedList } from "../libraries/LinkedList.sol"; import { OwnerManager } from "../managers/OwnerManager.sol"; import { ValidatorManager } from "../managers/ValidatorManager.sol"; diff --git a/src/helpers/TimestampAsserterLocator.sol b/src/helpers/TimestampAsserterLocator.sol index f01a666b..bdbb1387 100644 --- a/src/helpers/TimestampAsserterLocator.sol +++ b/src/helpers/TimestampAsserterLocator.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; -import "../interfaces/ITimestampAsserter.sol"; +import { ITimestampAsserter } from "../interfaces/ITimestampAsserter.sol"; library TimestampAsserterLocator { function locate() internal view returns (ITimestampAsserter) { diff --git a/src/helpers/TokenCallbackHandler.sol b/src/helpers/TokenCallbackHandler.sol index 4693f8d7..a8f6f7fa 100644 --- a/src/helpers/TokenCallbackHandler.sol +++ b/src/helpers/TokenCallbackHandler.sol @@ -1,8 +1,9 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.24; -import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; -import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; -import "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; + +import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import { IERC721Receiver } from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; +import { IERC1155Receiver } from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; /** * Token callback handler. diff --git a/src/libraries/JsmnSolLib.sol b/src/libraries/JsmnSolLib.sol index 83cd678b..63228113 100644 --- a/src/libraries/JsmnSolLib.sol +++ b/src/libraries/JsmnSolLib.sol @@ -31,10 +31,10 @@ library JsmnSolLib { PRIMITIVE } - uint256 constant RETURN_SUCCESS = 0; - uint256 constant RETURN_ERROR_INVALID_JSON = 1; - uint256 constant RETURN_ERROR_PART = 2; - uint256 constant RETURN_ERROR_NO_MEM = 3; + uint256 public constant RETURN_SUCCESS = 0; + uint256 public constant RETURN_ERROR_INVALID_JSON = 1; + uint256 public constant RETURN_ERROR_PART = 2; + uint256 public constant RETURN_ERROR_NO_MEM = 3; struct Token { JsmnType jsmnType; @@ -57,7 +57,7 @@ library JsmnSolLib { return (p, t); } - function allocateToken(Parser memory parser, Token[] memory tokens) internal pure returns (bool, Token memory) { + function allocateToken(Parser memory parser, Token[] memory tokens) private pure returns (bool, Token memory) { if (parser.toknext >= tokens.length) { // no more space in tokens return (false, tokens[tokens.length - 1]); @@ -68,7 +68,7 @@ library JsmnSolLib { return (true, token); } - function fillToken(Token memory token, JsmnType jsmnType, uint256 start, uint256 end) internal pure { + function fillToken(Token memory token, JsmnType jsmnType, uint256 start, uint256 end) private pure { token.jsmnType = jsmnType; token.start = start; token.startSet = true; @@ -77,7 +77,7 @@ library JsmnSolLib { token.size = 0; } - function parseString(Parser memory parser, Token[] memory tokens, bytes memory s) internal pure returns (uint) { + function parseString(Parser memory parser, Token[] memory tokens, bytes memory s) private pure returns (uint) { uint256 start = parser.pos; bool success; Token memory token; @@ -122,19 +122,23 @@ library JsmnSolLib { return RETURN_ERROR_PART; } - function parsePrimitive(Parser memory parser, Token[] memory tokens, bytes memory s) internal pure returns (uint) { + function parsePrimitive(Parser memory parser, Token[] memory tokens, bytes memory s) private pure returns (uint) { bool found = false; uint256 start = parser.pos; bool success; bytes1 c; Token memory token; + + // skip the first character because we assume we've already identified it as a primitive from parse + parser.pos++; + for (; parser.pos < s.length; parser.pos++) { c = s[parser.pos]; if (c == " " || c == "\t" || c == "\n" || c == "\r" || c == "," || c == 0x7d || c == 0x5d) { found = true; break; } - if (uint8(c) < 32 || uint8(c) > 127) { + if (uint8(c) < 32 || uint8(c) >= 127) { parser.pos = start; return RETURN_ERROR_INVALID_JSON; } @@ -162,7 +166,7 @@ library JsmnSolLib { (parser, tokens) = init(numberElements); // Token memory token; - uint256 r; + uint256 returnFlag; uint256 count = parser.toknext; uint256 i; Token memory token; @@ -170,7 +174,7 @@ library JsmnSolLib { for (; parser.pos < s.length; parser.pos++) { bytes1 c = s[parser.pos]; - // 0x7b, 0x5b opening curly parentheses or brackets + // 0x7b, 0x5b opening curly braces or square brackets if (c == 0x7b || c == 0x5b) { count++; (success, token) = allocateToken(parser, tokens); @@ -225,14 +229,13 @@ library JsmnSolLib { continue; } - // 0x42 + // 0x22 if (c == '"') { - r = parseString(parser, tokens, s); + returnFlag = parseString(parser, tokens, s); - if (r != RETURN_SUCCESS) { - return (r, tokens, 0); + if (returnFlag != RETURN_SUCCESS) { + return (returnFlag, tokens, 0); } - //JsmnError.INVALID; count++; if (parser.toksuper != -1) tokens[uint(parser.toksuper)].size++; continue; @@ -276,9 +279,9 @@ library JsmnSolLib { } } - r = parsePrimitive(parser, tokens, s); - if (r != RETURN_SUCCESS) { - return (r, tokens, 0); + returnFlag = parsePrimitive(parser, tokens, s); + if (returnFlag != RETURN_SUCCESS) { + return (returnFlag, tokens, 0); } count++; if (parser.toksuper != -1) { @@ -311,9 +314,8 @@ library JsmnSolLib { } // parseInt(parseFloat*10^_b) - function parseInt(string memory _a, uint256 _b) internal pure returns (int) { + function parseInt(string memory _a, uint256 _b) internal pure returns (int256 result) { bytes memory bresult = bytes(_a); - int256 mint = 0; bool decimals = false; bool negative = false; for (uint256 i = 0; i < bresult.length; i++) { @@ -325,13 +327,13 @@ library JsmnSolLib { if (_b == 0) break; else _b--; } - mint *= 10; - mint += int(int8(uint8(bresult[i]))) - 48; + result *= 10; + result += int(int8(uint8(bresult[i]))) - 48; } else if (uint8(bresult[i]) == 46) decimals = true; } - if (_b > 0) mint *= int(10 ** _b); - if (negative) mint *= -1; - return mint; + if (_b > 0) result *= int(10 ** _b); + if (negative) result *= -1; + return result; } function parseBool(string memory _a) internal pure returns (bool) { diff --git a/src/libraries/LinkedList.sol b/src/libraries/LinkedList.sol index f0b02739..0ec3ea43 100644 --- a/src/libraries/LinkedList.sol +++ b/src/libraries/LinkedList.sol @@ -3,169 +3,6 @@ pragma solidity ^0.8.24; import { Errors } from "../libraries/Errors.sol"; -/** - * @title Bytes linked list library - * @notice Helper library for bytes linkedlist operations - * @author https://getclave.io - */ -library BytesLinkedList { - bytes internal constant SENTINEL_BYTES = hex"00"; - uint8 internal constant SENTINEL_LENGTH = 1; - - modifier validBytes(bytes calldata value) { - if (value.length <= SENTINEL_LENGTH) { - revert Errors.INVALID_BYTES(value.length); - } - _; - } - - function add(mapping(bytes => bytes) storage self, bytes calldata value) internal validBytes(value) { - if (self[value].length != 0) { - revert Errors.BYTES_ALREADY_EXISTS(self[value]); - } - - bytes memory prev = self[SENTINEL_BYTES]; - if (prev.length < SENTINEL_LENGTH) { - self[SENTINEL_BYTES] = value; - self[value] = SENTINEL_BYTES; - } else { - self[SENTINEL_BYTES] = value; - self[value] = prev; - } - } - - function replace(mapping(bytes => bytes) storage self, bytes calldata oldValue, bytes calldata newValue) internal { - if (!exists(self, oldValue)) { - revert Errors.BYTES_NOT_EXISTS(oldValue); - } - if (exists(self, newValue)) { - revert Errors.BYTES_ALREADY_EXISTS(newValue); - } - - bytes memory cursor = SENTINEL_BYTES; - while (true) { - bytes memory _value = self[cursor]; - if (equals(_value, oldValue)) { - bytes memory next = self[_value]; - self[newValue] = next; - self[cursor] = newValue; - delete self[_value]; - return; - } - cursor = _value; - } - } - - function replaceUsingPrev( - mapping(bytes => bytes) storage self, - bytes calldata prevValue, - bytes calldata oldValue, - bytes calldata newValue - ) internal { - if (!exists(self, oldValue)) { - revert Errors.BYTES_NOT_EXISTS(oldValue); - } - if (exists(self, newValue)) { - revert Errors.BYTES_ALREADY_EXISTS(newValue); - } - if (!equals(self[prevValue], oldValue)) { - revert Errors.INVALID_PREV_BYTES(self[prevValue], oldValue); - } - - self[newValue] = self[oldValue]; - self[prevValue] = newValue; - delete self[oldValue]; - } - - function remove(mapping(bytes => bytes) storage self, bytes calldata value) internal { - if (!exists(self, value)) { - revert Errors.BYTES_NOT_EXISTS(value); - } - - bytes memory cursor = SENTINEL_BYTES; - while (true) { - bytes memory _value = self[cursor]; - if (equals(_value, value)) { - bytes memory next = self[_value]; - self[cursor] = next; - delete self[_value]; - return; - } - cursor = _value; - } - } - - function removeUsingPrev( - mapping(bytes => bytes) storage self, - bytes calldata prevValue, - bytes calldata value - ) internal { - if (!exists(self, value)) { - revert Errors.BYTES_NOT_EXISTS(value); - } - if (!equals(self[prevValue], value)) { - revert Errors.INVALID_PREV_BYTES(self[prevValue], value); - } - - self[prevValue] = self[value]; - delete self[value]; - } - - function clear(mapping(bytes => bytes) storage self) internal { - bytes memory cursor = SENTINEL_BYTES; - do { - bytes memory nextCursor = self[cursor]; - delete self[cursor]; - cursor = nextCursor; - } while (cursor.length > SENTINEL_LENGTH); - } - - function exists( - mapping(bytes => bytes) storage self, - bytes calldata value - ) internal view validBytes(value) returns (bool) { - return self[value].length != 0; - } - - function size(mapping(bytes => bytes) storage self) internal view returns (uint256) { - uint256 result = 0; - bytes memory cursor = self[SENTINEL_BYTES]; - while (cursor.length > SENTINEL_LENGTH) { - cursor = self[cursor]; - unchecked { - result++; - } - } - return result; - } - - function isEmpty(mapping(bytes => bytes) storage self) internal view returns (bool) { - return self[SENTINEL_BYTES].length <= SENTINEL_LENGTH; - } - - function list(mapping(bytes => bytes) storage self) internal view returns (bytes[] memory) { - uint256 _size = size(self); - bytes[] memory result = new bytes[](_size); - uint256 i = 0; - bytes memory cursor = self[SENTINEL_BYTES]; - while (cursor.length > SENTINEL_LENGTH) { - result[i] = cursor; - cursor = self[cursor]; - unchecked { - i++; - } - } - - return result; - } - - function equals(bytes memory a, bytes memory b) private pure returns (bool result) { - assembly { - result := eq(keccak256(add(a, 0x20), mload(a)), keccak256(add(b, 0x20), mload(b))) - } - } -} - /** * @title Address linked list library * @notice Helper library for address linkedlist operations diff --git a/src/managers/HookManager.sol b/src/managers/HookManager.sol index 20a0e6c0..b01ad763 100644 --- a/src/managers/HookManager.sol +++ b/src/managers/HookManager.sol @@ -159,7 +159,7 @@ abstract contract HookManager is IHookManager, Auth { } } - function _addHook(bytes calldata hookAndData, bool isValidation) internal { + function _addHook(bytes calldata hookAndData, bool isValidation) private { if (hookAndData.length < 20) { revert Errors.EMPTY_HOOK_ADDRESS(hookAndData.length); } @@ -187,7 +187,7 @@ abstract contract HookManager is IHookManager, Auth { emit AddHook(hookAddress); } - function _removeHook(address hook, bool isValidation) internal { + function _removeHook(address hook, bool isValidation) private { if (isValidation) { _validationHooksLinkedList().remove(hook); } else { @@ -233,7 +233,7 @@ abstract contract HookManager is IHookManager, Auth { hookDataStore = SsoStorage.layout().hookDataStore; } - function _supportsHook(address hook, bool isValidation) internal view returns (bool) { + function _supportsHook(address hook, bool isValidation) private view returns (bool) { return isValidation ? hook.supportsInterface(type(IValidationHook).interfaceId) diff --git a/src/managers/OwnerManager.sol b/src/managers/OwnerManager.sol index 99402e26..eabd6f2e 100644 --- a/src/managers/OwnerManager.sol +++ b/src/managers/OwnerManager.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.24; import { SsoStorage } from "../libraries/SsoStorage.sol"; -import { BytesLinkedList, AddressLinkedList } from "../libraries/LinkedList.sol"; +import { AddressLinkedList } from "../libraries/LinkedList.sol"; import { Errors } from "../libraries/Errors.sol"; import { Auth } from "../auth/Auth.sol"; import { ISsoAccount } from "../interfaces/ISsoAccount.sol"; @@ -16,8 +16,6 @@ import { IOwnerManager } from "../interfaces/IOwnerManager.sol"; * @author https://getclave.io */ abstract contract OwnerManager is IOwnerManager, Auth { - // Helper library for bytes to bytes mappings - using BytesLinkedList for mapping(bytes => bytes); // Helper library for address to address mappings using AddressLinkedList for mapping(address => address); @@ -57,7 +55,7 @@ abstract contract OwnerManager is IOwnerManager, Auth { return _k1OwnersLinkedList().exists(addr); } - function _k1OwnersLinkedList() internal view returns (mapping(address => address) storage k1Owners) { + function _k1OwnersLinkedList() private view returns (mapping(address => address) storage k1Owners) { k1Owners = SsoStorage.layout().k1Owners; } diff --git a/src/managers/ValidatorManager.sol b/src/managers/ValidatorManager.sol index 8f02a9c8..87a1c30d 100644 --- a/src/managers/ValidatorManager.sol +++ b/src/managers/ValidatorManager.sol @@ -26,8 +26,8 @@ abstract contract ValidatorManager is IValidatorManager, Auth { // Low level calls helper library using ExcessivelySafeCall for address; - function addModuleValidator(address validator, bytes memory initialAccountValidationKey) external onlySelf { - _addModuleValidator(validator, initialAccountValidationKey); + function addModuleValidator(address validator, bytes memory accountValidationKey) external onlySelf { + _addModuleValidator(validator, accountValidationKey); } ///@inheritdoc IValidatorManager @@ -56,7 +56,7 @@ abstract contract ValidatorManager is IValidatorManager, Auth { emit AddModuleValidator(validator); } - function _removeModuleValidator(address validator) internal { + function _removeModuleValidator(address validator) private { _moduleValidatorsLinkedList().remove(validator); validator.excessivelySafeCall(gasleft(), 0, abi.encodeWithSelector(IInitable.disable.selector)); @@ -68,7 +68,7 @@ abstract contract ValidatorManager is IValidatorManager, Auth { return _moduleValidatorsLinkedList().exists(validator); } - function _supportsModuleValidator(address validator) internal view returns (bool) { + function _supportsModuleValidator(address validator) private view returns (bool) { return validator.supportsInterface(type(IModuleValidator).interfaceId); } diff --git a/src/validators/SessionKeyValidator.sol b/src/validators/SessionKeyValidator.sol index 60ff8c79..384943c8 100644 --- a/src/validators/SessionKeyValidator.sol +++ b/src/validators/SessionKeyValidator.sol @@ -20,8 +20,6 @@ contract SessionKeyValidator is IModuleValidator { event SessionCreated(address indexed account, bytes32 indexed sessionHash, SessionLib.SessionSpec sessionSpec); event SessionRevoked(address indexed account, bytes32 indexed sessionHash); - bytes4 constant EIP1271_SUCCESS_RETURN_VALUE = 0x1626ba7e; - // account => number of open sessions // NOTE: expired sessions are still counted if not explicitly revoked mapping(address => uint256) private sessionCounter; @@ -40,12 +38,12 @@ contract SessionKeyValidator is IModuleValidator { } // This module should not be used to validate signatures - function validateSignature(bytes32 signedHash, bytes memory signature) external view returns (bool) { + function validateSignature(bytes32 signedHash, bytes memory signature) external pure returns (bool) { return false; } - function addValidationKey(bytes memory sessionData) external returns (bool) { - return _addValidationKey(sessionData); + function addValidationKey(bytes memory key) external returns (bool) { + return _addValidationKey(key); } function createSession(SessionLib.SessionSpec memory sessionSpec) public { diff --git a/src/validators/WebAuthValidator.sol b/src/validators/WebAuthValidator.sol index 457ac76f..f6053009 100644 --- a/src/validators/WebAuthValidator.sol +++ b/src/validators/WebAuthValidator.sol @@ -16,9 +16,9 @@ import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol /// @custom:security-contact security@matterlabs.dev /// @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; + address private constant P256_VERIFIER = address(0x100); + bytes1 private constant AUTH_DATA_MASK = 0x05; + bytes32 private constant lowSmax = 0x7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8; // The layout is weird due to EIP-7562 storage read restrictions for validation phase. mapping(string originDomain => mapping(address accountAddress => bytes32)) public lowerKeyHalf; @@ -36,7 +36,7 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { // so there's no way to just delete all the keys // We can only disconnect the module from the account, // re-linking it will allow any previous keys - function disable() external { + function disable() external pure { revert("Cannot disable module without removing it from account"); } @@ -160,14 +160,14 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { function _createMessage( bytes memory authenticatorData, bytes memory clientData - ) internal pure returns (bytes32 message) { + ) private 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) { + ) private pure returns (bytes memory authenticatorData, string memory clientDataSuffix, bytes32[2] memory rs) { (authenticatorData, clientDataSuffix, rs) = abi.decode(fatSignature, (bytes, string, bytes32[2])); }