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

withdrawInheritedFunds() does not check for address(0) slots

0xdonchev

Summary

When the owner calls InheritanceManager::removeBeneficiary and deletes an address of a beneficiary, the slot of the deleted address becomes address(0) and the length of the array InheritanceManager::beneficiaries stays the same.

Vulnerability Details

Having zero addresses in beneficiaries array could lead to serious issues in the InheritanceManager::withdrawInheritedFunds function. The function does not handle zero addresses because of which beneficiaries will receive less than expected funds.

Impact

When calling InheritanceManager::withdrawInheritedFunds, beneficiaries.length is used as a divisor, even if some slots are address(0). This causes funds being divided by an incorrect number. Real beneficiaries will receive less than expected and some funds will be send to address(0).

Tools Used

-manual analysis

Recommendations

Update InheritanceManager::withdrawInheritedFunds to check for address(0) and to use a correct divisor.

function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
​
+ uint256 noAddressZeroBeneficiaries = 0;
+ for (uint256 i = 0; i < beneficiaries.length; i++) {
+ if (beneficiaries[i] != address(0)) {
+ noAddressZeroBeneficiaries++;
+ }
+ }
+ require(noAddressZeroBeneficiaries > 0, "No valid beneficiaries");
- uint256 divisor = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
- uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
+ uint256 amountPerBeneficiary = ethAmountAvailable / noAddressZeroBeneficiaries;
- for (uint256 i = 0; i < divisor; i++) {
for (uint256 i = 0; i < beneficiaries.length; i++) {
+ if (beneficiaries[i] != address(0)) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
+ }
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
- uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
+ uint256 amountPerBeneficiary = assetAmountAvailable / noAddressZeroBeneficiaries;
- for (uint256 i = 0; i < divisor; i++) {
for (uint256 i = 0; i < beneficiaries.length; i++) {
+ if (beneficiaries[i] != address(0)) {
IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
+ }
}
}
}
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!