Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Incorrect deletion in `removeBeneficiary` Leaves gaps in beneficiaries array

Summary

The removeBeneficiary function in the InheritanceManager contract uses the delete keyword to remove a beneficiary from the beneficiaries array, which sets the targeted element to address(0) without reducing the array’s length or reorganizing its elements. This creates gaps in the array, leading to potential misbehavior in functions like inherit and withdrawInheritedFunds that rely on the array’s length and contents, such as incorrect fund distribution or unintended ownership changes.

Vulnerability Details

In the following removeBeneficiary function, delete is used without adjusting the array's length:

https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L163-L166

In Solidity, applying delete to an element of a dynamic array (e.g., delete beneficiaries[indexToRemove]) resets that element to its default value (address(0) for an address array) but does not adjust the array’s length or shift subsequent elements.

In the Inherit function which relies on beneficiaries.length If beneficiaries = [0x0] (after deleting the sole beneficiary), length == 1 still holds, allowing ownership transfer to msg.sender, even though the intended beneficiary is gone.
If beneficiaries = [0xA, 0x0, 0xC], length > 1 sets isInherited = true, despite a gap.

The funcion withdrawInheritedFunds which also relies on beneficiaries.length divides funds by beneficiaries.length and sends to each address in the array. If beneficiaries = [0xA, 0x0, 0xC], it sends funds to address(0) (burning ETH or tokens), reducing the amount received by valid beneficiaries (e.g., 10 ETH split three ways = 3.33 ETH each, with 3.33 ETH lost to 0x0).

Impact

In withdrawInheritedFunds, sending funds to address(0) effectively burns assets, reducing the share for legitimate beneficiaries and breaking the intended equal distribution.

In inherit, a length-based check might trigger incorrect behavior (e.g., ownership transfer or inheritance activation) even when the array contains invalid (address(0)) entries.

Tools Used

Manual code review

Recommendations

Replace the delete approach with a proper array removal technique that shifts elements and reduces length

Updates

Lead Judging Commences

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

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

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