Closed code423n4 closed 1 year ago
payloads are verified ahead of execution by the DAO, QA.
0xean changed the severity to QA (Quality Assurance)
0xean marked the issue as grade-c
Nullified this issue as this https://github.com/code-423n4/2023-03-aragon-findings/issues/167 contains the wardens information.
Lines of code
https://github.com/code-423n4/2023-03-aragon/blob/4db573870aa4e1f40a3381cdd4ec006222e471fe/packages/contracts/src/core/dao/DAO.sol#L186
Vulnerability details
Impact
In
DAO.sol
, the functionexecute
uses a low-level function.call
to perform_actions
:The code is executing a low-level call to a (potential) contract without verifying if the contract exists. If there is no code at the provided address, the call will succeed and the sender will lose the sent funds. According to the solidity docs: 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
https://docs.soliditylang.org/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions
REMIX IDE can be used to test PoC.
msg.value
will be lost.Tools Used
Manual Analysis
Recommended Mitigation Steps
Before executing the low-level call, it is important to verify the existence of the code. However, implementing this check may cause issues when transferring funds to an EOA address. To address this problem, we can prompt the user to specify whether the address is for an EOA or a contract. If the address is for an EOA, then the existence check is not necessary. However, if it is for a contract, the check is crucial to ensure the safety of the transaction. For that we need to add a
bool
in the structActions
:And then code existence check inside an
If
statement in the execute function: