From 28d9894cb567dff9d023116f02f8ff4d13346740 Mon Sep 17 00:00:00 2001 From: "david.ding" Date: Sat, 18 Nov 2023 19:30:10 +0800 Subject: [PATCH] Update Paymaster --- contracts/paymaster/ERC20Paymaster.sol | 35 +++++--- script/CreateWalletEntryPointPaymaster.s.sol | 8 +- .../ERC20PaymasterActiveWallet.t.sol | 88 ++++++++++++++++++- 3 files changed, 117 insertions(+), 14 deletions(-) diff --git a/contracts/paymaster/ERC20Paymaster.sol b/contracts/paymaster/ERC20Paymaster.sol index ffbb3a1c..c7523ceb 100644 --- a/contracts/paymaster/ERC20Paymaster.sol +++ b/contracts/paymaster/ERC20Paymaster.sol @@ -124,6 +124,11 @@ contract ERC20Paymaster is BasePaymaster { return (abi.encode(sender, token, costOfPost, exchangeRate), 0); } + /* + * @notice This function is currently in the testing phase. + * @dev The Paymaster is potentially vulnerable to attacks, which poses a risk of reputation loss. + * The approval check in this context does not guarantee that the Paymaster will successfully receive the corresponding tokens via transferFrom in subsequent _postOp operations. + */ function _validateConstructor(UserOperation calldata userOp, address token, uint256 tokenRequiredPreFund) internal view @@ -131,35 +136,45 @@ contract ERC20Paymaster is BasePaymaster { address factory = address(bytes20(userOp.initCode)); require(factory == WALLET_FACTORY, "Paymaster: unknown wallet factory"); require( - bytes4(userOp.callData) == bytes4(0x18dfb3c7 /* 0x18dfb3c7 executeBatch(address[],bytes[]) */ ), + /* + * 0x18dfb3c7 executeBatch(address[],bytes[]) + * 0x47e1da2a executeBatch(address[],uint256[],bytes[]) + */ + bytes4(userOp.callData) == bytes4(0x47e1da2a) || bytes4(userOp.callData) == bytes4(0x18dfb3c7), "invalid callData" ); - (address[] memory dest, bytes[] memory func) = abi.decode(userOp.callData[4:], (address[], bytes[])); - require(dest.length == func.length, "Paymaster: invalid callData length"); + address[] memory dest; + bytes[] memory func; + if (bytes4(userOp.callData) == bytes4(0x47e1da2a)) { + (dest,, func) = abi.decode(userOp.callData[4:], (address[], uint256[], bytes[])); + } else { + (dest, func) = abi.decode(userOp.callData[4:], (address[], bytes[])); + } + require(dest.length == func.length, "Paymaster: invalid callData length"); address _destAddress = address(0); + bool checkAllowance = false; for (uint256 i = 0; i < dest.length; i++) { address destAddr = dest[i]; require(isSupportToken(token), "Paymaster: token not support"); - if (destAddr == token) { + // check it contains approve operation, 0x095ea7b3 approve(address,uint256) + if (destAddr == token && bytes4(func[i]) == bytes4(0x095ea7b3)) { (address spender, uint256 amount) = _decodeApprove(func[i]); require(spender == address(this), "Paymaster: invalid spender"); - require(amount >= tokenRequiredPreFund, "Paymaster: snot enough approve"); + require(amount >= tokenRequiredPreFund, "Paymaster: not enough approve"); + checkAllowance = true; + break; } - require(destAddr > _destAddress, "Paymaster: duplicate"); - _destAddress = destAddr; } + require(checkAllowance, "no approve found"); // callGasLimit uint256 callGasLimit = dest.length * _SAFE_APPROVE_GAS_COST; require(userOp.callGasLimit >= callGasLimit, "Paymaster: gas too low for postOp"); } function _decodeApprove(bytes memory func) private pure returns (address spender, uint256 value) { - // 0x095ea7b3 approve(address,uint256) // 0x095ea7b3 address uint256 // ____4_____|____32___|___32__ - - require(bytes4(func) == bytes4(0x095ea7b3), "invalid approve func"); assembly { spender := mload(add(func, 36)) // 32 + 4 value := mload(add(func, 68)) // 32 + 4 +32 diff --git a/script/CreateWalletEntryPointPaymaster.s.sol b/script/CreateWalletEntryPointPaymaster.s.sol index 09801479..47a53ca9 100644 --- a/script/CreateWalletEntryPointPaymaster.s.sol +++ b/script/CreateWalletEntryPointPaymaster.s.sol @@ -102,10 +102,14 @@ contract CreateWalletEntryPointPaymaster is Script { tokenAddressList[0] = address(payToken); + uint256[] memory values = new uint256[](1); + values[0] = 0; + bytes[] memory tokenCallData = new bytes[](1); tokenCallData[0] = abi.encodeWithSignature("approve(address,uint256)", address(paymaster), 1000e6); - bytes memory callData = - abi.encodeWithSignature("executeBatch(address[],bytes[])", tokenAddressList, tokenCallData); + bytes memory callData = abi.encodeWithSignature( + "executeBatch(address[],uint256[],bytes[])", tokenAddressList, values, tokenCallData + ); bytes memory paymasterAndData = abi.encodePacked(abi.encodePacked(address(paymaster)), abi.encode(address(payToken), uint256(1000e6))); diff --git a/test/paymaster/ERC20PaymasterActiveWallet.t.sol b/test/paymaster/ERC20PaymasterActiveWallet.t.sol index 0987c29d..99b64235 100644 --- a/test/paymaster/ERC20PaymasterActiveWallet.t.sol +++ b/test/paymaster/ERC20PaymasterActiveWallet.t.sol @@ -72,7 +72,7 @@ contract ERC20PaymasterActiveWalletTest is Test, UserOpHelper { vm.stopPrank(); } - function testActiveWalletUsingPaymaster() external { + function test_ActiveWalletUsingPaymaster() external { address sender; uint256 nonce; bytes memory initCode; @@ -119,9 +119,14 @@ contract ERC20PaymasterActiveWalletTest is Test, UserOpHelper { address[] memory tokenAddressList = new address[](1); tokenAddressList[0] = address(token); + uint256[] memory values = new uint256[](1); + values[0] = 0; + bytes[] memory tokenCallData = new bytes[](1); tokenCallData[0] = abi.encodeWithSignature("approve(address,uint256)", address(paymaster), 1000e6); - callData = abi.encodeWithSignature("executeBatch(address[],bytes[])", tokenAddressList, tokenCallData); + callData = abi.encodeWithSignature( + "executeBatch(address[],uint256[],bytes[])", tokenAddressList, values, tokenCallData + ); paymasterAndData = abi.encodePacked(abi.encodePacked(address(paymaster)), abi.encode(address(token), uint256(1000e6))); @@ -144,4 +149,83 @@ contract ERC20PaymasterActiveWalletTest is Test, UserOpHelper { soulWallet = ISoulWallet(sender); assertEq(soulWallet.isOwner(ownerAddr.toBytes32()), true); } + + function test_ActiveWalletWithoutApprove() external { + address sender; + uint256 nonce; + bytes memory initCode; + bytes memory callData; + uint256 callGasLimit; + uint256 verificationGasLimit; + uint256 preVerificationGas; + uint256 maxFeePerGas; + uint256 maxPriorityFeePerGas; + bytes memory paymasterAndData; + bytes memory signature; + + nonce = 0; + + (address trustedManagerOwner,) = makeAddrAndKey("trustedManagerOwner"); + TrustedModuleManager trustedModuleManager = new TrustedModuleManager(trustedManagerOwner); + TrustedPluginManager trustedPluginManager = new TrustedPluginManager(trustedManagerOwner); + SecurityControlModule securityControlModule = + new SecurityControlModule(trustedModuleManager, trustedPluginManager); + + bytes[] memory modules = new bytes[](1); + modules[0] = abi.encodePacked(securityControlModule, abi.encode(uint64(2 days))); + bytes[] memory plugins = new bytes[](0); + + bytes32 salt = bytes32(0); + bytes32[] memory owners = new bytes32[](1); + owners[0] = ownerAddr.toBytes32(); + DefaultCallbackHandler defaultCallbackHandler = new DefaultCallbackHandler(); + bytes memory initializer = abi.encodeWithSignature( + "initialize(bytes32[],address,bytes[],bytes[])", owners, defaultCallbackHandler, modules, plugins + ); + sender = soulWalletFactory.getWalletAddress(initializer, salt); + // send wallet with testtoken + token.sudoMint(address(sender), 1000e6); + bytes memory soulWalletFactoryCall = abi.encodeWithSignature("createWallet(bytes,bytes32)", initializer, salt); + initCode = abi.encodePacked(address(soulWalletFactory), soulWalletFactoryCall); + + verificationGasLimit = 2000000; + preVerificationGas = 500000; + maxFeePerGas = 10 gwei; + maxPriorityFeePerGas = 10 gwei; + callGasLimit = 3000000; + + address[] memory tokenAddressList = new address[](1); + tokenAddressList[0] = address(token); + + uint256[] memory values = new uint256[](1); + values[0] = 0; + + bytes[] memory tokenCallData = new bytes[](1); + tokenCallData[0] = abi.encodeWithSignature("allowance(address,uint256)", address(paymaster), 1000e6); + callData = abi.encodeWithSignature( + "executeBatch(address[],uint256[],bytes[])", tokenAddressList, values, tokenCallData + ); + paymasterAndData = + abi.encodePacked(abi.encodePacked(address(paymaster)), abi.encode(address(token), uint256(1000e6))); + + UserOperation memory userOperation = UserOperation( + sender, + nonce, + initCode, + callData, + callGasLimit, + verificationGasLimit, + preVerificationGas, + maxFeePerGas, + maxPriorityFeePerGas, + paymasterAndData, + signature + ); + + userOperation.signature = signUserOp(userOperation, ownerKey); + vm.expectRevert( + abi.encodeWithSelector(bytes4(keccak256("FailedOp(uint256,string)")), 0, "AA33 reverted: no approve found") + ); + bundler.post(entryPoint, userOperation); + } }