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

Function "buyOutEstateNFT" transfers incorrect amount of assets to beneficiaries

al88nsk

Summary

Function "buyOutEstateNFT" incorrectly calculates amount of assets to transfer to each beneficiary.

Vulnerability Details

According the documentation, the contract must calculate amounts to transfer using the logic below:

If the beneficiaries settle the NFTs on-chain the amount to pay is (Value / Number Of Beneficiaries) * (Number Of Beneficiaries - 1) since the paying beneficiary does not need to pay his own share. The above calculation is equally distributed between the other beneficiaries.

The actual calculation is as folloes:

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);
}

Here finalAmount is the amount that excludes the paying beneficiary and it is distributed among other beneficiaries, i.e. each beneficiary must receive finalAmount/multiplier assets. However, current implementation uses divisor instead of multiplier, and each beneficiary receives less amount of assets than it is intended.

Impact

Beneficiaries receive less assets than they must receive.

Tools Used

Manual review

Recommendations

Change the transferred amount of tokens:

-IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
+IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / multiplier);
Updates

Lead Judging Commences

0xtimefliez Lead Judge 10 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!