code-423n4 / 2023-06-llama-findings

2 stars 1 forks source link

The low-level call returns true if the address does not exist #276

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-llama/blob/main/src/accounts/LlamaAccount.sol#L323-L326 https://github.com/code-423n4/2023-06-llama/blob/main/src/LlamaExecutor.sol#L34

Vulnerability details

Impact

As written in the Solidity documentation (https://docs.soliditylang.org/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions), the call, delegatecall and staticcall low-level functions return true as its first return value if the account being invoked does not exist, as part of the design of the EVM. The existence of the account must be verified prior to calling, if necessary.

Proof of Concept

The low-level call and delegatecall functions are used in some places in the code, and this can be problematic. For example, in the execute() contract of LlamaAccount, there is a low-level call, but if the given target does not exist, success will be true and the function will return true, and code execution will continue as the call was successful.

Tools Used

Recommended Mitigation Steps

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

Assessed type

call/delegatecall

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #183

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Out of scope