HardhatFoundry
30,000 USDC
View results
Submission Details
Severity: high
Valid

ETH is not forwarded to factory call in BiconomyMetaFactory

Summary

The BiconomyMetaFactory contract doesn't forward any potential received ETH to the underlying factory call.

Vulnerability Details

The BiconomyMetaFactory contract works as a meta-deployer of accounts. It has a deployWithFactory() function that takes a factory and executes an inner call to create the account.

70: function deployWithFactory(address factory, bytes calldata factoryData) external payable returns (address payable createdAccount) {
71: require(factoryWhitelist[address(factory)], FactoryNotWhitelisted());
72: (bool success, bytes memory returnData) = factory.call(factoryData);
73:
74: // Check if the call was successful
75: require(success, CallToDeployWithFactoryFailed());
76:
77: // Decode the returned address
78: assembly {
79: createdAccount := mload(add(returnData, 0x20))
80: }
81: }

The function is marked as payable, indicating it may receive ETH. However, there is no handling of the potential received value. ETH is not forwarded to the inner factory and would be stuck in the contract.

Note that this is an intended use case, since factories expect to receive ETH to create the account. For example, in the NexusAccountFactory contract the createAccount() function is marked as payable and can be used to send ETH to the created account using the LibClone.createDeterministicERC1967() utility function.

44: function createAccount(bytes calldata initData, bytes32 salt) external payable override returns (address payable) {
45: // Compute the actual salt for deterministic deployment
46: bytes32 actualSalt;
47: assembly {
48: let ptr := mload(0x40)
49: let calldataLength := sub(calldatasize(), 0x04)
50: mstore(0x40, add(ptr, calldataLength))
51: calldatacopy(ptr, 0x04, calldataLength)
52: actualSalt := keccak256(ptr, calldataLength)
53: }
54:
55: // Deploy the account using the deterministic address
56: (bool alreadyDeployed, address account) = LibClone.createDeterministicERC1967(msg.value, ACCOUNT_IMPLEMENTATION, actualSalt);

Impact

ETH sent to the deployWithFactory() function is not forwarded to the underlying factory and would be left unused in the BiconomyMetaFactory contract, causing loss of funds.

Tools Used

None.

Recommendations

Forward the callvalue to the inner factory call.

- (bool success, bytes memory returnData) = factory.call(factoryData);
+ (bool success, bytes memory returnData) = factory.call{value: msg.value}(factoryData);
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-cannot-msg.value-not-forwarded

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.