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

Array Manipulation - Removal of Beneficiary Without Proper Cleanup

Summary

The function removeBeneficiary in the InheritanceManager contract does not properly handle array manipulation when removing a beneficiary. The current implementation deletes the beneficiary from the beneficiaries array but does not compact the array to avoid gaps. This can lead to issues with functions that rely on the correct order and integrity of the array, such as the withdrawInheritedFunds function, which expects a correct list of beneficiaries to distribute funds.

Vulnerability Details

In the removeBeneficiary function, the removal of a beneficiary from the beneficiaries array is done by simply deleting the element at the given index:

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

However, this does not handle the array properly as it leaves a “hole” in the array, meaning the elements after the deleted beneficiary are not shifted, potentially causing misalignment in the array.

This issue affects functions that rely on the correct list of beneficiaries, especially the withdrawInheritedFunds function, which divides available assets among the beneficiaries. If the array is not properly updated, the distribution logic may behave unexpectedly, as the function iterates over an incomplete or misaligned list of beneficiaries.

Impact

• Incorrect Fund Distribution: The withdrawInheritedFunds function assumes that the beneficiaries array is properly ordered and free from gaps. If beneficiaries are removed without proper array cleanup, the function may miscalculate the amount of funds to be distributed, causing assets to be incorrectly allocated or skipped.

• Array Integrity Compromised: The integrity of the beneficiaries array is compromised, which could lead to unexpected behavior when interacting with other functions that rely on it, such as onlyBeneficiaryWithIsInherited.

Tools Used

Manual code review

Recommendations

• Array Compaction: When removing a beneficiary, ensure that the array is compacted to prevent any gaps. A typical solution is to replace the removed beneficiary with the last element in the array and then call pop to remove the last element, like so:

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
beneficiaries[indexToRemove] = beneficiaries[beneficiaries.length - 1];
beneficiaries.pop();
}
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!