Failed transfer with low level call could be overlooked

[code423n4]

Lines of code

https://github.com/ERC725Alliance/ERC725/blob/7171a0e25e83cfe4c4dec6262bb62b4422c0478f/implementations/contracts/ERC725XCore.sol#L177 https://github.com/ERC725Alliance/ERC725/blob/7171a0e25e83cfe4c4dec6262bb62b4422c0478f/implementations/contracts/ERC725XCore.sol#L194 https://github.com/ERC725Alliance/ERC725/blob/7171a0e25e83cfe4c4dec6262bb62b4422c0478f/implementations/contracts/ERC725XCore.sol#L211

Vulnerability details

Impact

Transfers may fail silently.

Proof of Concept

• 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".
• For reference, see a similar high severity reported in a Uniswap audit here (report # 9): https://github.com/Uniswap/v3-core/blob/main/audits/tob/audit.pdf

  File: ERC725XCore
  177:        (bool success, bytes memory returnData) = target.call{value: value}(data);
  ----
  194:        (bool success, bytes memory returnData) = target.staticcall(data);
  ---- 
  211:        (bool success, bytes memory returnData) = target.delegatecall(data);

  ERC725XCore#_executeCall ERC725XCore#_executeStaticCall ERC725XCore#_executeDelegateCall

Tools Used

Manual Review

Recommended Mitigation Steps

Check for the account's existence prior to transferring.

Assessed type

Token-Transfer

[c4-pre-sort]

minhquanym marked the issue as primary issue

[minhquanym]

OOS in winning bot race

[c4-pre-sort]

minhquanym marked the issue as low quality report

[c4-judge]

trust1995 marked the issue as unsatisfactory: Out of scope