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

[H-4] Miscalculation in InheritanceManager::buyOutEstateNFT Leads to Incorrect Beneficiary Distribution and Violates Protocol Invariant

agmguindo

Description:

The function InheritanceManager::buyOutEstateNFT() is responsible for allowing a beneficiary to buy out a Real Estate NFT by paying its total value, which is then distributed among the other beneficiaries. However, there is a miscalculation in the way funds are distributed:

The function incorrectly uses finalAmount / divisor to determine the share for each beneficiary. Instead, it should use beneficiaries.length - 1, because the buyer is not supposed to receive a share. This results in an underpayment to each beneficiary.

​
function buyOutEstateNFT(
uint256 _nftID
) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
// @audit-MEdium: losing precision. always need to multiply before divide in solidity.
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 impact overall is High beacuse afect the lost of funds for the Beneficiaries and at the same time the likelihood of the vulnerability is High as it will always happen.

Proof of Concept:

Example:

NFT Value: $100

Number of Beneficiaries: 4

Expected Share per Beneficiary: $100 / 4 = $25 each

Current Incorrect Calculation: $100 / 4 × 3 = $75 (distributed among 3)

Actual Amount Received per Beneficiary: $75 / 4 = $18.75

This breaks the protocol's invariant that each non-buying beneficiary must receive an equal and fair share.

Poc 
```javascript

 function test_audit_Return_added_to_loop() public {
    vm.deal(address(im), 10e18);
    vm.startPrank(owner);
    string memory _description = "pepe";
    uint256 _value = 1000000;
    address _asset = address(usdc);
    im.createEstateNFT(_description, _value, _asset);
    uint256 valueOfNft = im.getNftValue(1);
    console.log(valueOfNft);
    console.log(nft.ownerOf(1));

    im.addBeneficiery(address(user1));
    im.addBeneficiery(address(user2));
    im.addBeneficiery(address(user3));
    im.addBeneficiery(address(user4));

    uint256 deadLine2 = im.getDeadline();
    console.log(deadLine2);
    vm.warp(deadLine2 + 90 days);
    vm.stopPrank();

    vm.startPrank(user4);
    im.inherit();
    im.withdrawInheritedFunds(address(0));
    usdc.mint(address(user4), 10e6);
    usdc.approve(address(im), type(uint256).max);

    im.buyOutEstateNFT(1);
    vm.stopPrank();
    
}
```

Recommended Mitigation:
To mitigate this calculation error, just divide by beneficiaries.length - 1 and the distribution will be right

​
function buyOutEstateNFT(
uint256 _nftID
) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
// @audit-MEdium: losing precision. always need to multiply before divide in solidity.
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
+ finalAmount / multiplier
);
}
}
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

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

buyOutNFT has wrong denominator

buyOutNFT has return instead of continue

Support

FAQs

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

Give us feedback!