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

Incorrect calculation in buyOutEstateNFT(), InheritanceManager.sol

Summary

The buyOutEstateNFT function contains a critical issue in the following line:

uint256 multiplier = beneficiaries.length - 1;

Vulnerability Details

If beneficiaries.length is 1, then multiplier = 1 - 1 = 0, which means the finalAmount will always be 0, leading to no funds being transferred.

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

Impact

The finalAmount will always be zero, resulting in no funds being transferred to the other beneficiaries

Tools Used

Manual review

Recommendations

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
require(divisor > 1, "Cannot buy out if only one beneficiary exists"); // new line
uint256 finalAmount = (value * (divisor - 1)) / divisor;
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
uint256 share = finalAmount / (divisor - 1); // new line
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (beneficiaries[i] != msg.sender) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], share);
}
}
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

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