0xdcota
commented
1 year ago Case 2 was addressed by checking receiver is not zero address, which would effectively be burning ETH if such error is done. However, case 1 is user's due diligence, There is no effective way to distinguish between an EOA wallet, or a self-destructed contract, but cases will return data = 0x0.
Affected smart contract:
BaseRouter.sol
Severity:
Medium
Impact:
users ETH assets can be lost due to user error. I classify it as medium because assets can be lost due to user error.
Description:
users eth assets can be lost while user is withdrawing their ETH using
xBundle()https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/abstracts/BaseRouter.sol#L271-L279
while executing WithdrawETH Action, receiver can be contract address also but it is not checking that contract is exist or not.
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".
Scenario 2: not checking that receiver should be non zero address ETH can be lost due to user error, best practice is to check that receiver is not zero address.
Recommended Mitigation Steps:
Check for if receiver is contract check existence on low-level calls and add check receiver should be non zero, so that failures are not missed.