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

Improper Beneficiary Array Management Leading to Gaps and Calculation Errors

0xbyteknight

Summary

The InheritanceManager contract uses the delete operator to remove beneficiaries from the beneficiaries array, which leaves zero address gaps. These gaps can lead to erroneous fund distribution calculations and may even create denial-of-service (DoS) scenarios when the array is processed.

Vulnerability Details

When a beneficiary is removed using the delete operator, the beneficiary's slot in the array is set to the zero address rather than being removed entirely. As a result, subsequent operations that iterate over the beneficiaries array—for example, splitting funds equally during withdrawal—will include these zero addresses. This can cause incorrect division of funds, as the total number of beneficiaries used in the calculation does not reflect the actual intended recipients. In certain scenarios, the presence of these gaps could also be exploited to trigger unexpected behavior or even a DoS when processing the array.

Impact

Direct Impact: Incorrect fund splits can result, where valid beneficiaries receive less than their fair share.

Indirect Impact: In extreme cases, if the array is processed without filtering out zero addresses, it could lead to a denial-of-service (DoS) situation, disrupting the distribution of funds.

Tools Used

Manual code review

Foundry (Forge) unit tests to simulate beneficiary removal and fund distribution calculations

Recommendations

Replace the use of delete with a more robust mechanism for managing beneficiary removals. For instance, implement an approach that either shifts array elements to maintain a compact array or use a mapping to track active beneficiaries.

If retaining an array structure, ensure that functions iterating over beneficiaries explicitly skip zero addresses.

PoC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
contract InheritanceManagerBeneficiaryGapTest is Test {
InheritanceManager im;
ERC20Mock usdc;
​
uint256 public constant INITIAL_BALANCE = 1000e18;
​
address owner = makeAddr("owner");
address beneficiary1 = makeAddr("beneficiary1");
address beneficiary2 = makeAddr("beneficiary2");
address beneficiary3 = makeAddr("beneficiary3");
​
uint256 public constant TOKEN_AMOUNT = 9e18;
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
usdc = new ERC20Mock();
vm.deal(address(usdc), INITIAL_BALANCE);
}
​
/// @notice Demonstrates that using `delete` to remove a beneficiary leaves a zero address gap.
/// Also verifies that the beneficiaries array length remains unchanged.
function test_withdrawInheritedFundsERC20WithRemovedBeneficiary() public {
// Arrange: Add three beneficiaries.
vm.startPrank(owner);
im.addBeneficiery(beneficiary1);
im.addBeneficiery(beneficiary2);
im.addBeneficiery(beneficiary3);
vm.stopPrank();
​
// Remove beneficiary2.
vm.prank(owner);
im.removeBeneficiary(beneficiary2);
// At this point, the beneficiaries array still has length 3,
// but the slot for beneficiary2 is now the zero address.
​
// Verify the beneficiaries array length remains 3.
// We assume the beneficiaries array is stored at slot 2.
uint256 beneficiaryLength = uint256(
vm.load(address(im), bytes32(uint256(5)))
);
assertEq(
beneficiaryLength,
3,
"Beneficiaries array length should be 3"
);
​
// Warp time to meet the 90-day inactivity requirement.
vm.warp(1 + 90 days);
// Trigger inheritance; since there are multiple beneficiaries, `inherit()` sets isInherited to true.
vm.prank(beneficiary1);
im.inherit();
​
// Mint USDC tokens to the InheritanceManager.
usdc.mint(address(im), TOKEN_AMOUNT);
​
// Act & Assert: When withdrawInheritedFunds is called,
// the distribution loop calculates amountPerBeneficiary = TOKEN_AMOUNT / 3.
// However, one transfer will attempt to send tokens to address(0), which should revert.
vm.prank(beneficiary1);
vm.expectRevert(); // Expect revert because safeTransfer to address(0) fails.
im.withdrawInheritedFunds(address(usdc));
}
}
​
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.