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

Incorrect Division of Payment in buyOutEstateNFT() Function

Summary

There is a critical issue in the InheritanceManager.sol::buyOutEstateNFT() function where the amount being transferred to beneficiaries is incorrectly calculated. The calculation of the transfer amount should account for the fact that the caller already owns part of the NFT and does not need to receive a share of the payment.

Vulnerability Details

Affected code:

The loop in the buyOutEstateNFT() function currently calculates the transfer amount for each beneficiary by dividing the finalAmount by the total number of beneficiaries, even when the caller (who is one of the beneficiaries) is the one purchasing the NFT.

Consider the following example:

Let’s consider an estate NFT priced at 100,000 USDC, with 3 beneficiaries. If one of the beneficiaries (say Alice) buys the NFT, they are only responsible for paying the remaining amount.

  • The caller should pay 66,000 USDC (since one third of the NFT is already theirs).

  • The other two beneficiaries should receive 33,000 USDC each.

However, due to the current calculation (finalAmount / divisor), the transfer amount is computed as:

66,000 / 3 = 22,000 USDC (for each of the other two beneficiaries).

This results in each beneficiary receiving 22,000 USDC instead of the correct amount of 33,000 USDC.

Impact

The financial impact of this error is significant, as the beneficiaries will receive an incorrect payment. Specifically, each beneficiary will receive less than their entitled share. In the example above, each of the other two beneficiaries should receive 33,000 USDC, but they would only receive 22,000 USDC. This could lead to financial disputes, and the system would not function as expected. Although the remaining funds are not necessarily lost if owner loses control over his wallet (unlikely) or withdrawInheritedFunds() is called then the left 22,000 USDC (only 44,000 USDC transferred among the left beneficiaries) will be divided among all beneficiaries equally so 22,000 USDC / 3 = 7,333 USDC. At the end, the other two beneficiaries (following the example above) will receive a total of 29,333 USDC which is still less than the deserved 33,000 USDC. Additionally, the buyer of the NFT in the particular case will receive 7,333 USDC which do not belong to them anymore.

Tools Used

  • Manual review

Recommendations

To fix this issue, the transfer logic should account for the caller already owning a share of the NFT. Instead of dividing by divisor (the total number of beneficiaries), it should divide by divisor - 1 (excluding the caller from the payment distribution). This would ensure that the correct amount is paid to the remaining beneficiaries.

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

Lead Judging Commences

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