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

Token Precision Loss Leading to Misallocated Funds

ghufranhassan1

Summary

In this contract there is risk of losing funds due to precision loss and all beneficiaries might not get equal share

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);
}
}
nft.burnEstate(_nftID);
}
​

Vulnerability Details

Let us assume these values for calculation

value = 1000 USDC

  • beneficiaries = ["Alice", "Bob", "Charlie", "Dave"] (4 total)

  • divisor = 4 (total beneficiaries)

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

finalAmount = (value / divisor) * multiplier;

(1000/4)×3=250×3 = 750 USDC

The final amount will be 750 USDC

IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);

Transfer per beneficiary would be 750/4​=187.5 USDC

But Solidity only supports integers, so 187.5 gets truncated to 187.

  • msg.sender (buyer) pays 750 USDC

  • Each beneficiary receives 187 USDC

  • Total distributed: 187 × 3 = 561 USDC

  • There is 3 micro USDC loss due to truncation which is as follows
    750−561=189 micro USDC

    i.e. Lost 189 micro-USDC (189 * 1e-6 USDC), equivalent to 0.189 USDC

Impact

Beneficiaries are underpaid due to rounding off error and incurring loss for beneficiaries.



Tools Used

Manual Review

Recommendations

Use dynamic scaling we can achieve the results as follows. It will support both USDC and DAI and there would be no loss in token precision.

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
​
// Get token decimals dynamically
+ uint8 decimals = IERC20Metadata(assetToPay).decimals();
+uint256 scaleFactor = 10**(18 - decimals); // Adjust for different tokens
​
// Scale the value correctly
+ uint256 scaledValue = value * scaleFactor;
+ uint256 finalAmount = (scaledValue / divisor) * multiplier;
+ IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount / scaleFactor);
+ uint256 amountPerBeneficiary = (finalAmount / multiplier) / scaleFactor;
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
return;
} else {
+ IERC20(assetToPay).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

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