Biconomy: Nexus
Biconomy
HardhatFoundry
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

deployWithFactory() doesn't passes ethers to the deployed contract which will result in stuck Ethers in the BiconomyMetaFactory contract

Shaheen

Summary

BiconomyMetaFactory.deployWithFactory() doesn't passes ethers to the deployed contract which will result in stuck Ethers in the BiconomyMetaFactory contract.

Vulnerability Details

BiconomyMetaFactory's depolyWithFactoryfunction allows users to invoke any factories' like NexusAccountFactory's createAccount function, which deploys a smart account contract using createDeterministicERC1967 or CREATE2:

function deployWithFactory(address factory, bytes calldata factoryData) external payable returns (address payable createdAccount) {
require(factoryWhitelist[address(factory)], FactoryNotWhitelisted());
@> (bool success, bytes memory returnData) = factory.call(factoryData);
​
// Check if the call was successful
require(success, CallToDeployWithFactoryFailed());
​
// Decode the returned address
assembly {
createdAccount := mload(add(returnData, 0x20))
}
}

NexusAccountFactory.createAccount():

function createAccount(bytes calldata initData, bytes32 salt) external payable override returns (address payable) {
// Compute the actual salt for deterministic deployment
bytes32 actualSalt;
assembly {
let ptr := mload(0x40)
let calldataLength := sub(calldatasize(), 0x04)
mstore(0x40, add(ptr, calldataLength))
calldatacopy(ptr, 0x04, calldataLength)
actualSalt := keccak256(ptr, calldataLength)
}
// Deploy the account using the deterministic address
@> (bool alreadyDeployed, address account) = LibClone.createDeterministicERC1967(msg.value, ACCOUNT_IMPLEMENTATION, actualSalt);
​
if (!alreadyDeployed) {
INexus(account).initializeAccount(initData);
emit AccountCreated(account, initData, salt);
}
return payable(account);
}

Notice that the createDeterministicERC1967 or CREATE2 method is used to deploy the contract. The CREATE2 method is known for deploying and funding the contract. As we can see thats exactly what happens in the createAccount, the createAccount function is a payable function and it passes all the msg.value to the createDeterministicERC1967/CREATE2 but same is not the case with deployWithFactory

Issue

The issue is that, as we can see the deployWithFactory is a paybale function, which will call another contract's payable function createAccount but when it .call that function, it doesn't passes the msg.value to that function in the call. Means any ethers/msg.value sent by the users (for the deploying contract) will be not passed to the createAccount/CREATE2 and the ethers will be stuck in the contract forever.

function deployWithFactory(address factory, bytes calldata factoryData) external payable returns (address payable createdAccount) {
require(factoryWhitelist[address(factory)], FactoryNotWhitelisted());
///@audit-issue M doesn't passes the msg.value! - Any eth sent will be locked in this contract!
@> (bool success, bytes memory returnData) = factory.call(factoryData);
.....
}

Proof-of-Concept

Add this test in the Nexus.Factory.specs.ts file and run yarn hardhat test --grep "Should deploy Nexus account with Ethers stuck in the metaFactory PoC"

it("Should deploy Nexus account with Ethers stuck in the metaFactory PoC", async function () {
await metaFactory.addFactoryToWhitelist(await factory.getAddress());
const salt = keccak256("0x");
const initData = await bootstrap.getInitNexusScopedCalldata(
[parsedValidator],
parsedHook,
registry,
[],
0,
);
const factoryData = factory.interface.encodeFunctionData(
"createAccount",
[initData, salt],
);
expect(await ethers.provider.getBalance(metaFactory)).to.be.equal(0,);
​
await expect(
metaFactory.deployWithFactory(await factory.getAddress(), factoryData, {value: to18(1)}),
).to.emit(factory, "AccountCreated");
​
expect(await ethers.provider.getBalance(metaFactory)).to.be.equal(to18(1),);
});
Compiled 106 Solidity files successfully (evm target: paris).
​
​
Nexus Factory Tests
Biconomy Meta Factory tests
✔ Should deploy Nexus account with Ethers stuck in the metaFactory PoC (41ms)
​
​
1 passing (55s)

Impact

  • Loss of Funds for the users

  • Ethers stuck in a contract forever as their is no way to fetch tokens from the BiconomyMetaFactory contract

Recommendations

Make BiconomyMetaFactory.deployWithFactory() non-payable or pass the msg.value:

function deployWithFactory(address factory, bytes calldata factoryData) external payable returns (address payable createdAccount) {
require(factoryWhitelist[address(factory)], FactoryNotWhitelisted());
(bool success, bytes memory returnData) = factory.call{value: msg.value}(factoryData);
​
// Check if the call was successful
require(success, CallToDeployWithFactoryFailed());
​
// Decode the returned address
assembly {
createdAccount := mload(add(returnData, 0x20))
}
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year 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.