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

Division Before multiplication

suitec9

Summary

A bug exists in the buyOutEstateNFT function of the InheritanceManager contract. The function performs integer division before multiplication when calculating payment amounts, resulting in precision loss due to truncation. This allows users to purchase estate NFTs at lower prices than intended, causing financial loss to the protocol and beneficiaries.

Vulnerability Details

In Solidity, when performing integer division, any fractional results are truncated (rounded down). If division is performed before multiplication, this truncation error gets amplified, potentially leading to significant discrepancies in financial calculations.

The vulnerable code in the buyOutEstateNFT function:

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
// ... rest of the function
}

The function calculates the payment amount by first dividing the NFT value by the number of beneficiaries and then multiplying by one less than the number of beneficiaries. This order of operations leads to potential value loss due to integer division truncation.

Impact

This vulnerability allows users to purchase NFTs at reduced prices, causing:

  1. Direct Financial Loss: Beneficiaries receive less compensation than intended for the estate NFT.

  2. Economic Imbalance: The protocol consistently undervalues assets, leading to systemic underpricing.

  3. Exploitation Potential: Malicious users can target NFTs with values that maximize the truncation error.

Proof of Concept

Consider an NFT valued at 3,000,001 tokens with 3 beneficiaries:

Current Implementation (Vulnerable):

value = 3,000,001
divisor = 3
multiplier = 2
finalAmount = (3,000,001 / 3) * 2
= 1,000,000 * 2
= 2,000,000

Correct Implementation:

value = 3,000,001
divisor = 3
multiplier = 2
finalAmount = (3,000,001 * 2) / 3
= 6,000,002 / 3
= 2,000,000.67
= 2,000,000 (after truncation)

The discrepancy in this example is only 0.67 tokens, but with larger values and certain combinations of divisors and multipliers, the impact can be substantial.

test output

NFT value is 3-000-000

├─ [55920] InheritanceManager::buyOutEstateNFT(1)
│ ├─ [26058] 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f::transferFrom(user4: [0x90561e5Cd8025FA6F52d849e8867C14A77C94BA0], InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 1500000 [1.5e6])

Tools Used

Recommendations

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value * multiplier) / divisor; // Fixed order of operations
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
// ... rest of function
}

Additional Considerations

While this fix addresses the immediate issue, consider implementing additional safeguards:

  1. Add thorough documentation explaining the formula and its purpose

  2. Consider using a fixed-point arithmetic library for enhanced precision

  3. Add sufficient tests that verify correct calculation with various input values

  4. Implement event emissions to log all payment calculations for transparency

By addressing this vulnerability, the protocol ensures fair pricing for estate NFTs and protects both the beneficiaries and the integrity of the inheritance system.

Updates

Lead Judging Commences

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

truncation of integers

Appeal created

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

truncation of integers

Support

FAQs

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

Give us feedback!