Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

Rounding Precision Error in `inheritanceManager::buyOutEstateNFT` function leads to loss of funds

Summary

A rounding issue in the inheritanceManager::buyOutEstateNFT function causes incorrect fund transfer due to integer division errors.

Vulnerability Details

vulnerable code

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
console.log(value);
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);
}
}
nft.burnEstate(_nftID);
}

More Explanation

  • The calculation of finalAmount involves integer division (value / divisor), which causes truncation.

  • Example: If value = 300003 and beneficiaries.length = 4, the expected finalAmount should be 225002, but due to truncation, it results in 225000.

Proof of Concept

POC

Paste the following test in the inheritanceManagerTest.t.sol file.

function testRoundingIssue() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
address user4 = makeAddr("user4");
uint256 value = 300003; // 3e5 + 3
vm.warp(1);
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.addBeneficiery(user4);
im.createEstateNFT("our beach-house", value, address(usdc));
vm.stopPrank();
usdc.mint(user3, 4e6);
vm.warp(1 + 90 days);
vm.startPrank(user3);
usdc.approve(address(im), 4e6);
im.inherit();
im.buyOutEstateNFT(1);
vm.stopPrank();
uint256 divisor = 4; // beneficiaries.length
uint256 multiplier = 3; // beneficiaries.length - 1
uint256 expectedFinalAmount = (value * multiplier) / divisor; // 225002
uint256 incorrectFinalAmount = (value / divisor) * multiplier; // 225000
console.log("Expected final amount:", expectedFinalAmount);
console.log("Incorrect final amount:", incorrectFinalAmount);
}

Impact

  • Over time, miscalculations could accumulate into significant lost funds.

Tools Used

  • Foundry

Recommendations

  • Use multiplication before division to prevent truncation:

    uint256 finalAmount = (value * multiplier) / divisor;
  • Implement precision handling by using a scaling factor (e.g., value * 1e18 and then dividing at the end).

Updates

Lead Judging Commences

0xtimefliez Lead Judge 5 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.