The InheritanceManager.sol::withdrawInheritedFunds() function calculates the amount of ERC20 tokens or ETH to distribute to beneficiaries. However, if a beneficiary was removed by the owner before withdrawing inherited funds then the amount of Native ETH will be calculated incorrectly.
Affected code:
To calculate how much share of the inherited funds each beneficiary can receive the function does the following calculation => uint256 amountPerBeneficiary = ethAmountAvailable / divisor;. However, the current implementation does not account for when a beneficiary was previously removed by the owner.
The main problem of the current implementation is that the delete keyword in Solidity does not reduce the array’s length - it only resets the specified index to address(0), leaving a “hole” in the array. Since the contract relies on beneficiaries.length for fund distribution, this will always lead to incorrect calculations when inheritance is involved.
Example:
Owner deposits 90 ETH to the contract.
Owner adds 3 beneficiaries to the contract. [0x01, 0x02, 0x03]
After some time the owner removes one of them. [0x01, 0x00, 0x03]
After the contract is inherited beneficiaries withdraw funds.
Instead of dividing by 2 the function will divide by 3 and each user will receive only 30 ETH. (correct amount is 45 ETH)
Note that demonstrating incorrect calculations in PoC when transferring native ETH is impossible to write because the contract does not have payable functions to receive native ETH.
Incorrect ETH allocation between beneficiaries.
Note that funds are not lost (remain in the contract and can be retrieved) but a part of the core functionality of the protocol is broken.
Manual review
Use the pop() method of the dynamic array in Solidity and swap the last element with the removed one.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.