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

"Array Deletion Ghost Entry Vulnerability Leading to Permanent Fund Loss"

jayx

Summary

A critical vulnerability has been identified in the InheritanceManager smart contract, specifically in the beneficiary management system. The removeBeneficiary() function implementation creates "ghost entries" in the beneficiaries array when removing addresses. These ghost entries (set to address(0)) continue to be counted during fund distribution, causing a portion of inheritance funds to be sent to the zero address, resulting in permanent loss of assets.

Vulnerability Details

The vulnerability stems from the improper implementation of array element removal in the removeBeneficiary() function:

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
}

The function uses Solidity's delete operator, which doesn't remove the element from the array but instead resets it to its default value (address(0)). This creates the following issues:

  1. The beneficiaries array maintains its original length even after elements are removed

  2. The array contains "ghost entries" (zero addresses) where beneficiaries were removed

  3. These ghost entries are still counted in calculations and iterations

Below is the test case that demonstrates this vulnerability:

POC (PROOF OF CONCEPT)
ADD THIS GETTER FUNCTION IN InheritanceManager.sol

function getBeneficiaries() public view returns (address[] memory) {
return beneficiaries;
}

Below is the test file for the vulnerability:

//SPDX-License-Identifier: MIT
​
pragma solidity 0.8.26;
​
import {Test, console} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
contract InheritanceManagerTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
​
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
usdc = new ERC20Mock();
weth = new ERC20Mock();
}
​
function testGhostEntryIssue() public {
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
vm.stopPrank();
//get the initial beneficiaries
address[] memory initialBeneficiaries = im.getBeneficiaries();
assertEq(initialBeneficiaries.length, 3);
//removing one beneficiary
vm.prank(owner);
im.removeBeneficiary(user2);
// Get updated benefeciaries
address[] memory updatedBeneficiaries = im.getBeneficiaries();
// The array should still have 3 elements
assertEq(updatedBeneficiaries.length, 3);
// But the middle one should now be address(0) - a "ghost" entry
assertEq(updatedBeneficiaries[0], user1);
assertEq(updatedBeneficiaries[1], address(0)); // This is the ghost entry!
assertEq(updatedBeneficiaries[2], user3);
// Log for visual inspection
console.log("Beneficiaries after removal with current implementation:");
for (uint i = 0; i < updatedBeneficiaries.length; i++) {
console.log("Index", i, ":", updatedBeneficiaries[i]);
}
// Now we demonstrate how this affects fund distribution
// First, we need to fund the address and advance time to enable inheritance
vm.deal(address(im), 9e18);
vm.warp(block.timestamp + 90 days);
vm.startPrank(user1);
im.inherit();
im.withdrawInheritedFunds(address(0));
vm.stopPrank();
// Now try to withdraw funds - this would typically send tokens to the zero address
assertEq(3e18, user1.balance);
assertEq(0, user2.balance);
assertEq(3e18, user3.balance);
console.log("In a real scenario, withdrawInheritedFunds would try to send tokens to address(0)");
console.log("user2 balance", user2.balance);
}
​
}
​

As demonstrated in the test, when calculating distributions, the function:

  • Uses the full length of the array (including ghost entries) as the divisor

  • Attempts to send funds to all entries in the array, including address(0)

  • Results in 1/3 of the funds (3 ETH in the test) being sent to the zero address and permanently lost

Impact

Direct Financial Loss: A portion of inheritance funds equal to (total funds / number of beneficiaries) is permanently lost for each removed beneficiary, as it's sent to the zero address.
Misleading Fund Distribution: Remaining legitimate beneficiaries receive less than their expected share since funds are incorrectly divided.

Tools Used
foundry/manual review

Recommendations
Array Filtering

function removeBeneficiary(address _beneficiary) external onlyOwner {
address[] memory newBeneficiaries = new address[](beneficiaries.length - 1);
uint256 j = 0;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (beneficiaries[i] != _beneficiary) {
newBeneficiaries[j] = beneficiaries[i];
j++;
}
}
beneficiaries = newBeneficiaries;
_setDeadline();
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 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.

Give us feedback!