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

buyOutEstateNFT Function issues

xyizko

Summary

​
​
​
`buyOutEstateNFT` Function issues
​
Problematic code
```solidity
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

  1. Incomplete beneficiary payout due to looping logic flaw

  2. Incorrect fun calculation (Truncation of remainder amounts)

  3. Due to incorrect looping the nft.burnEstate is never reached, function can be called multiple times

Impact

Impact 1 - Due to the loop termination flaw, When the sender is the beneficiary the loop exits and safe transfer isnt executed

Impact 2

.
.
uint256 finalAmount = (value / divisor) * multiplier;
.
.
​

  • Due to truncation of remainder amouts, incorrect amounts will be distributed

Impact 3 - Due to Impact 1 the function nft.burnEstate(_nftID); is never reached with sender == beneficiary this can be used as a replay attach since the this function can be called multiple times, and the nft never gets burned

Tools Used

  1. Manual review of math logic

  2. AI for understanding impact and seriousness

Recommendations

Fix 1 : Final amount calculation should take into account of remainder amounts

Fix 2 : Loop should still contiune in the event the sender == benficiary instead of exiting

Fix 3 : If fix 2 is implemented then Replay Attack issue will be resolved

​
Updates

Lead Judging Commences

0xtimefliez Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

buyOutNFT has return instead of continue

truncation of integers

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.