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

Removing a Beneficiary Makes the Contract Non-Inheritable

ssalaudeen10

Summary

The InheritanceManager.sol contract is designed to allow the owner to add new beneficiaries and remove existing ones from the list/array of beneficiaries. However, Solidity does not remove or delete slots in an array. Instead, the slot is replaced with address(0), which is the default behavior. When the owner calls InheritanceManager::removeBeneficiary, the address provided as a parameter is intended to be deleted from the array but is replaced with address(0). This causes issues with transferring inherited assets, as the presence of address(0) is not considered a valid transfer.

Vulnerability Details

The following test case demonstrates the issue

Impact

Inheritance assets in the contract becomes non-transferable when a beneficiary slot is replaced by address(0)

function test_lost_of_funds_due_to_empty_array()public{
// Create User2 to be added as a beneficiary
address user2 = makeAddr("user2");
vm.startPrank(owner);
// Add both user1 and user2 as a beneficiary
im.addBeneficiery(user1);
im.addBeneficiery(user2);
// Remove user2 from the list of beneficiary
im.removeBeneficiary(user2);
// Mint 10 usdc to the Inheritance Manager contract
usdc.mint(address(im), 10 ether);
// Increase the timestamp to the expected time lock
vm.warp(block.timestamp + im.TIMELOCK() + 1 days);
// Inherit the Inheritance Manager contract
im.inherit();
/**
Trying to withdraw the inherited USDC balance to the beneficiary which should be only user1 but deleting user2 only replace user to address with zero address. Inherited USDC would not be transferable because one of the beneficiary is address 0
*/
vm.expectRevert();
im.withdrawInheritedFunds(address(usdc));
vm.stopPrank();
}
​

Tools Used

Foundry test

Recommendations

Create a function that filters out address(0) from the list of beneficiaries.

  1. create a function that filters out address zero from the list of beneficiaries

  2. Use this filtered list in InheritanceManager::inherit, InheritanceManager::withdrawInheritedFunds, and InheritanceManager::buyOutEstateNFT to get the correct list of available beneficiaries

contract InheritanceManager is Trustee{
​
+ function _filterBeneficiaries()internal view returns(address[] memory ){
+ uint256 listOfBeneficiaries = beneficiaries.length;
+ uint numberOfNoneAddressZero;
+ for(uint i =0; i < listOfBeneficiaries; i++){
+ if(beneficiaries[i] != address(0)){
+ numberOfNoneAddressZero ++;
+ }
+ }
+ address[] memory filteredAddressList = new address[](numberOfNoneAddressZero);
+ uint256 uniqueIndex;
+ for(uint i =0; i < listOfBeneficiaries; i++){
+ if(beneficiaries[i] != address(0)){
+ filteredAddressList[uniqueIndex] = beneficiaries[i];
+ uniqueIndex ++;
+ }
+ }
+ return filteredAddressList;
+ }
​
function inherit() external {
+ uint256 adjustedBeneficiaryLength = _filterBeneficiaries();
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
+ if (adjustedBeneficiaryLength.length == 1) {
+ owner = adjustedBeneficiaryLength[0];
_setDeadline();
+ } else if (adjustedBeneficiaryLength.length > 1) {
isInherited = true;
+ _setDeadline();
​
} else {
revert InvalidBeneficiaries();
}
​
}
​
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
+ address[] memory adjustedBeneficiaries =_filterBeneficiaries();
+ uint256 divisor = adjustedBeneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
+ address payable beneficiary = payable(adjustedBeneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
+ IERC20(_asset).safeTransfer(adjustedBeneficiaries[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!