Inheritable Smart Contract Wallet
First Flight #35
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

[L-1] `InheritanceManager::sendERC20` and `InheritanceManager::sendETH` does not follow CEI pattern

0xnick

Description:

A best practice in order to prevent from different vectors of attack is to follow the Check-Effects-Interactions (CEI) pattern. This is marked as low due to a low possibility attack as it needs high level of coordination.

Impact:

A possible vector attack may arise, if we interacted with a malicious contract and the deadline period has
passed, then the ERC-20 contract or receiving contract can call our InheritanceManager::inherit function flipping the InheritanceManager::isInherited flag to true, allowing beneficiaries to take the assets from the contract.

Proof of Concept:

Steps for sendERC20:

  1. Create malicious ERC-20 contract. On transfer also check if the deadline has passed and call
    InheritanceManager::inherit function.

  2. Owner uses InheritanceManager::sendERC20 and the malicious ERC-20 flips the flag InheritanceManager::isInherited.

Recommended Mitigation:

Follow CEI pattern:

function sendERC20(address _tokenAddress, uint256 _amount, address _to) external nonReentrant onlyOwner {
if (IERC20(_tokenAddress).balanceOf(address(this)) < _amount) {
revert InsufficientBalance();
}
+ _setDeadline();
IERC20(_tokenAddress).safeTransfer(_to, _amount);
- _setDeadline();
}
function sendETH(uint256 _amount, address _to) external nonReentrant onlyOwner {
(bool success,) = _to.call{value: _amount}("");
+ _setDeadline();
require(success, "Transfer Failed");
- _setDeadline();
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.