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

Funds Distribution Error in `buyOutEstateNFT` Function

suitec9

Summary

A critical bug exists in the buyOutEstateNFT function of the InheritanceManager contract. The function incorrectly distributes funds to beneficiaries, resulting in significant portions of tokens becoming permanently locked in the contract. This implementation error leads to direct financial loss for beneficiaries and creates an imbalance in the inheritance system.

Vulnerability Details

The buyOutEstateNFT function contains a flawed distribution mechanism that incorrectly calculates the amount each beneficiary should receive. After transferring the full finalAmount to the contract, the function then distributes finalAmount / divisor to each beneficiary (except the buyer). This results in:

  1. Double division of the original payment amount

  2. Incorrect distribution of funds

  3. A substantial portion of tokens becoming permanently locked in the contract

The vulnerable code:

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
return;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor); // bug
}
}
nft.burnEstate(_nftID);
}

The key issue is that after calculating finalAmount, the function further divides this amount by divisor when distributing to each beneficiary, essentially applying division twice to the original value.

Impact

This vulnerability has severe financial implications:

  1. Permanent Fund Lock: Significant portions of tokens become permanently stranded in the contract with no recovery mechanism.

  2. Beneficiary Loss: Beneficiaries receive much less than their intended share of the inheritance.

  3. Protocol Imbalance: The locked funds create an economic imbalance in the protocol.

  4. Scaling Issue: The problem worsens as the number of beneficiaries increases.

Proof of Concept

Consider an estate NFT worth 3,000,000 tokens with 2 beneficiaries:

  1. value = 3,000,000

  2. divisor = 2 (beneficiaries)

  3. multiplier = 1 (beneficiaries.length - 1)

  4. finalAmount = (3,000,000 / 2) * 1 = 1,500,000

The buyer transfers 1,500,000 tokens to the contract. Then in the distribution loop:

  • Each beneficiary (excluding the buyer) receives: finalAmount / divisor = 1,500,000 / 2 = 750,000

  • With 1 non-buyer beneficiary, 750,000 tokens are distributed

  • Result: 750,000 tokens (50% of the payment) remain locked in the contract permanently

Test Case Evidence

The following test output confirms this vulnerability:

│ ├─ [26058] 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f::transferFrom(user4: [0x90561e5Cd8025FA6F52d849e8867C14A77C94BA0], InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 1500000 [1.5e6])
│ │ ├─ emit Transfer(from: user4: [0x90561e5Cd8025FA6F52d849e8867C14A77C94BA0], to: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], value: 1500000 [1.5e6])
│ │ └─ ← [Return] true
│ ├─ [25204] 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f::transfer(user3: [0xc0A55e2205B289a967823662B841Bd67Aa362Aec], 750000 [7.5e5])
│ │ ├─ emit Transfer(from: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], to: user3: [0xc0A55e2205B289a967823662B841Bd67Aa362Aec], value: 750000 [7.5e5])
│ │ └─ ← [Return] true
│ └─ ← [Stop]

With more beneficiaries, the problem compounds:

For 5 beneficiaries and a 1,000,000 token NFT:

  • finalAmount = (1,000,000 / 5) * 4 = 800,000

  • Each non-buyer receives: 800,000 / 5 = 160,000

  • Total distributed: 160,000 * 4 = 640,000

  • Locked funds: 800,000 - 640,000 = 160,000 (20% of payment)

Code Snippet

* CAN NOT use ETHER
* @param _nftID NFT ID to buy out
*/
function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
return;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor); // Vulnerable line
}
}
nft.burnEstate(_nftID);
}

Recommenadtions

Alternatively, a simpler solution if the compensation should be equal among non-buying beneficiaries:

uint256 value = nftValue[_nftID];
uint256 beneficiaryCount = beneficiaries.length;
uint256 nonBuyerCount = beneficiaryCount - 1;
// Calculate total payment amount
uint256 finalAmount = (value * nonBuyerCount) / beneficiaryCount;
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
// Calculate amount per recipient
uint256 amountPerBeneficiary = finalAmount / nonBuyerCount;
// Distribute funds
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender != beneficiaries[i]) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
nft.burnEstate(_nftID);
}

Tools used

Manual review
foundry test setup

Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

buyOutNFT has wrong denominator

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!