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 depolyWithFactory
function 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);
require(success, CallToDeployWithFactoryFailed());
assembly {
createdAccount := mload(add(returnData, 0x20))
}
}
NexusAccountFactory.createAccount()
:
function createAccount(bytes calldata initData, bytes32 salt) external payable override returns (address payable) {
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)
}
@> (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());
@> (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
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);
require(success, CallToDeployWithFactoryFailed());
assembly {
createdAccount := mload(add(returnData, 0x20))
}
}