diff --git a/audit-reports/preliminary-audits/airdroperc20-claimable.md b/audit-reports/preliminary-audits/airdroperc20-claimable.md new file mode 100644 index 000000000..aa22f074b --- /dev/null +++ b/audit-reports/preliminary-audits/airdroperc20-claimable.md @@ -0,0 +1,11 @@ +This document contains details on fixes / response to the preliminary audit reports added to this repository. + +## [AirdropERC20Claimable](./airdroperc20-claimable.pdf) + +### 01: Governance: TrustedForwarder can execute claims on behalf of other addresses + +- The contract doesn't add a trusted-forwarder address by default. The deployer of AirdropERC20Claimable can specify which forwarder they want to use (if any), or leave as address zero. + +### 02: Malicious users can steal the entire balance of the contract + +- This refers to the possibility of a sybil attack on open/public claims, where multiple wallets can be created to claim the quantity specified by `openClaimLimitPerWallet`. To prevent this scenario or any kind of public claiming, deployer can set `openClaimLimitPerWallet` to zero when setting claim conditions during deployment. diff --git a/audit-reports/preliminary-audits/airdroperc20-claimable.pdf b/audit-reports/preliminary-audits/airdroperc20-claimable.pdf new file mode 100644 index 000000000..64a02af70 Binary files /dev/null and b/audit-reports/preliminary-audits/airdroperc20-claimable.pdf differ diff --git a/contracts/package.json b/contracts/package.json index 66d4d968a..4cdb18f0d 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,7 +1,7 @@ { "name": "@thirdweb-dev/contracts", "description": "Collection of smart contracts deployable via the thirdweb SDK, dashboard and CLI", - "version": "3.11.0", + "version": "3.11.1", "license": "Apache-2.0", "repository": { "type": "git", diff --git a/contracts/prebuilts/account/non-upgradeable/Account.sol b/contracts/prebuilts/account/non-upgradeable/Account.sol index adc4bb3e3..e8cea3243 100644 --- a/contracts/prebuilts/account/non-upgradeable/Account.sol +++ b/contracts/prebuilts/account/non-upgradeable/Account.sol @@ -62,13 +62,18 @@ contract Account is AccountCore, ContractMetadata, ERC1271, ERC721Holder, ERC115 super.supportsInterface(interfaceId); } - /// @notice See EIP-1271 + /** + * @notice See EIP-1271 + * + * @param _hash The original message hash of the data to sign (before mixing this contract's domain separator) + * @param _signature The signature produced on signing the typed data hash (result of `getMessageHash(abi.encode(rawData))`) + */ function isValidSignature( - bytes32 _message, + bytes32 _hash, bytes memory _signature ) public view virtual override returns (bytes4 magicValue) { - bytes32 messageHash = getMessageHash(abi.encode(_message)); - address signer = messageHash.recover(_signature); + bytes32 targetDigest = getMessageHash(_hash); + address signer = targetDigest.recover(_signature); if (isAdmin(signer)) { return MAGICVALUE; @@ -89,12 +94,13 @@ contract Account is AccountCore, ContractMetadata, ERC1271, ERC721Holder, ERC115 /** * @notice Returns the hash of message that should be signed for EIP1271 verification. - * @param message Message to be hashed i.e. `keccak256(abi.encode(data))` - * @return Hashed message + * @param _hash The message hash to sign for the EIP-1271 origin verifying contract. + * @return messageHash The digest to sign for EIP-1271 verification. */ - function getMessageHash(bytes memory message) public view returns (bytes32) { - bytes32 messageHash = keccak256(abi.encode(MSG_TYPEHASH, keccak256(message))); - return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), messageHash)); + function getMessageHash(bytes32 _hash) public view returns (bytes32) { + bytes32 messageHash = keccak256(abi.encode(_hash)); + bytes32 typedDataHash = keccak256(abi.encode(MSG_TYPEHASH, messageHash)); + return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), typedDataHash)); } /*/////////////////////////////////////////////////////////////// diff --git a/contracts/prebuilts/account/utils/AccountExtension.sol b/contracts/prebuilts/account/utils/AccountExtension.sol index c9aad670e..b2ceca2b7 100644 --- a/contracts/prebuilts/account/utils/AccountExtension.sol +++ b/contracts/prebuilts/account/utils/AccountExtension.sol @@ -64,13 +64,18 @@ contract AccountExtension is ContractMetadata, ERC1271, AccountPermissions, ERC7 super.supportsInterface(interfaceId); } - /// @notice See EIP-1271 + /** + * @notice See EIP-1271 + * + * @param _hash The original message hash of the data to sign (before mixing this contract's domain separator) + * @param _signature The signature produced on signing the typed data hash (result of `getMessageHash(abi.encode(rawData))`) + */ function isValidSignature( - bytes32 _message, + bytes32 _hash, bytes memory _signature ) public view virtual override returns (bytes4 magicValue) { - bytes32 messageHash = getMessageHash(abi.encode(_message)); - address signer = messageHash.recover(_signature); + bytes32 targetDigest = getMessageHash(_hash); + address signer = targetDigest.recover(_signature); if (isAdmin(signer)) { return MAGICVALUE; @@ -91,12 +96,13 @@ contract AccountExtension is ContractMetadata, ERC1271, AccountPermissions, ERC7 /** * @notice Returns the hash of message that should be signed for EIP1271 verification. - * @param message Message to be hashed i.e. `keccak256(abi.encode(data))` - * @return Hashed message + * @param _hash The message hash to sign for the EIP-1271 origin verifying contract. + * @return messageHash The digest to sign for EIP-1271 verification. */ - function getMessageHash(bytes memory message) public view returns (bytes32) { - bytes32 messageHash = keccak256(abi.encode(MSG_TYPEHASH, keccak256(message))); - return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), messageHash)); + function getMessageHash(bytes32 _hash) public view returns (bytes32) { + bytes32 messageHash = keccak256(abi.encode(_hash)); + bytes32 typedDataHash = keccak256(abi.encode(MSG_TYPEHASH, messageHash)); + return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), typedDataHash)); } /*/////////////////////////////////////////////////////////////// diff --git a/src/test/smart-wallet/AccountVulnPOC.t.sol b/src/test/smart-wallet/AccountVulnPOC.t.sol index 6ffd8b6ad..b9087f70d 100644 --- a/src/test/smart-wallet/AccountVulnPOC.t.sol +++ b/src/test/smart-wallet/AccountVulnPOC.t.sol @@ -279,7 +279,7 @@ contract SimpleAccountVulnPOCTest is BaseTest { // However they can bypass this by using signature verification on number contract instead vm.prank(accountSigner); bytes32 digest = keccak256(abi.encode(42)); - bytes32 toSign = SimpleAccount(payable(account)).getMessageHash(abi.encode(digest)); + bytes32 toSign = SimpleAccount(payable(account)).getMessageHash(digest); (uint8 v, bytes32 r, bytes32 s) = vm.sign(accountSignerPKey, toSign); bytes memory signature = abi.encodePacked(r, s, v);