Biconomy: Nexus
Biconomy
HardhatFoundry
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`_isContract(...)` check may be bypassed

x18a6

Summary

_isContract(...) check may be bypassed to install modules on malicious accounts.

Vulnerability Details

The code in question is K1Validator::_isContract(...):

function _isContract(address account) private view returns (bool) {
uint256 size;
assembly {
size := extcodesize(account)
}
return size > 0;
}

The code checks if account is a contract. It examines the code size of its address. However, if a call is made to this function from within a contract's constructor, _isContract(account) returns false despite account being a contract.

Impact

If all the installation logic is crafted inside a constructor,_isContract(...) check may be bypassed:

function onInstall(bytes calldata data) external {
...
address newOwner = address(bytes20(data));
require(!_isContract(newOwner), NewOwnerIsContract()); //@audit newOwner code size is 0 at creation time
smartAccountOwners[msg.sender] = newOwner;
}

Therefore, a smart contract may own a smart account, undermining K1Validator trust.

Tools Used

Manual review.

Recommendations

function onInstall(bytes calldata data) external {
...
address newOwner = address(bytes20(data));
+ require(msg.sender == tx.origin, "Error: no contract allowed");
require(!_isContract(newOwner), NewOwnerIsContract());
smartAccountOwners[msg.sender] = newOwner;
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

finding-isContract-check

Invalid [known issue [Medium-3]](https://github.com/Cyfrin/2024-07-biconomy/issues/1)

Support

FAQs

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

Give us feedback!