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

Array Manipulation Vulnerability in removeBeneficiary

0xblackadam

Description:
The removeBeneficiary() function uses delete beneficiaries[indexToRemove] which sets the address at that index to the zero address but doesn't actually remove the element from the array. This leaves a gap in the array, which could cause issues with array length calculations and iteration logic.

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

Impact:

  • The function does not properly remove beneficiaries, potentially affecting the inheritance distribution logic

  • The zero address could be considered a valid beneficiary in some operations

  • The beneficiaries.length will still include deleted entries, affecting calculations like equal distribution of assets

  • Funds lost to zero address

Recommendation:
Implement proper array removal by swapping the element to be removed with the last element and then popping the last element:

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
uint256 lastIndex = beneficiaries.length - 1;
if (indexToRemove != lastIndex) {
beneficiaries[indexToRemove] = beneficiaries[lastIndex];
}
beneficiaries.pop();
_setDeadline();
}

Tools Used

  • Foundry Testing Framework

  • Manual Code Review

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.