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

Deletion of beneficiary using InheritanceManager:removedBeneficiary() can make the Funds stucked within the contract leading to denial of service

docweb3

Summary

Deleting beneficairy after it has been added will be replaced by the default address in solidity which is 0x00. Once the contract has been inherited, the 'InheritanceManager:withdrawInheritedFunds()' will not be able to distribute the funds due to the presence of an invalid address which is the default address that replaced the deleted address. This is so because use of deletion does not change the length of array and only replaced the element of the array with a default value which is 0x00 incase of address in solidity

Vulnerability Details

1.After adding three users

2.Delete one of the users

3.Inherit and then withdraw the funds after deadline . Copy the code below

4.The Function will revert instead of distributing the funds to ths users

function test_withdrawInheritedFundsHasFundStuckedAfterDeletion() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.removeBeneficiary(user2);
vm.stopPrank();
vm.warp(1);
usdc.mint(address(im), 9e18);
vm.warp(1 + 90 days);
vm.startPrank(user1);
im.inherit();
vm.expectRevert();
im.withdrawInheritedFunds(address(usdc));
vm.stopPrank();
}

Then run : forge test --mt test_withdrawInheritedFundsHasFundStuckedAfterDeletion

The output will be:

Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_withdrawInheritedFundsHasFundStuckedAfterDeletion() (gas: 221081)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.02ms (397.40µs CPU time)

Impact

The Funds are going to be stucked within the contract leading to denial of service as the funds can not be withdrawn from the contracts

Tools Used : Fondry

Recommendations

Instead of using deletion in https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L163

replace it with

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
require(indexToRemove < beneficiaries.length, "Beneficiary not found");
​
// Shift elements to fill the gap
for (uint256 i = indexToRemove; i < beneficiaries.length - 1; i++) {
beneficiaries[i] = beneficiaries[i + 1];
}
​
// Remove the last element (which is now a duplicate)
beneficiaries.pop();
}
​

After this paste the code below in inheritanceManager.t.sol function

function test_withdrawInheritedFundsAfterDeletion() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.removeBeneficiary(user2);
vm.stopPrank();
vm.warp(1);
usdc.mint(address(im), 9e18);
vm.warp(1 + 90 days);
vm.startPrank(user1);
im.inherit();
im.withdrawInheritedFunds(address(usdc));
vm.stopPrank();
assertEq(4.5e18, usdc.balanceOf(user1));
assertEq(4.5e18, usdc.balanceOf(user3));
}

and Run this

forge test --mt test_withdrawInheritedFundsAfterDeletion

The output will be

Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_withdrawInheritedFundsAfterDeletion() (gas: 230086)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.91ms (718.30µs CPU time)

​
​
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect removal from beneficiary list causes funds to be send to 0 address

Support

FAQs

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