code-423n4 / 2023-01-biconomy-findings

13 stars 10 forks source link

LOW LEVEL CALL RETURNS TRUE IF THE ADDRESS DOESN’T EXIST #467

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/common/SecuredTokenTransfer.sol#L22 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/libs/MultiSend.sol#L53-L56

Vulnerability details

As written in the solidity documentation, the low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed.

Proof of Concept

The low-level functions call and delegatecall are used in some places in the code and it can be problematic. For example, in the transferToken of the SecuredTokenTransfer contract there is a low level call in order to call the transfer function, but if the given token address doesn’t exist, success will be equal to true and the function will return true and the code execution will be continued like the call was successful.

File: common/SecuredTokenTransfer.sol

22:      let success := call(sub(gas(), 10000), token, 0, add(data, 0x20), mload(data), 0, 0x20)

Link to Code

Similarly in MultiSend.sol, if to address doesn’t exist, the delegate call or call will return true and the function won’t revert.

File: libs/MultiSend.sol

53:      success := call(gas(), to, value, data, dataLength, 0, 0)

56:      success := delegatecall(gas(), to, data, dataLength, 0, 0)

Link to Code

Recommended Mitigation Steps

Check before any low-level call that the address actually exists, for example before the low level call in the SecuredTokenTransfer function you can check that the address is a contract by checking its code size.

To Learn more Checkout similar Issue Reported here

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Insufficient proof