Description:
The function buyOutEstateNFT() includes a return statement inside a for loop, which causes an early exit when the sender is the first beneficiary in the array. This breaks the distribution logic, as it prevents the remaining beneficiaries from receiving their share of the funds.
It breaks the invariant of the protocol: The above calculation is equally distributed between the other beneficiaries.
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:
Likelihood: High – If the sender is the first beneficiary, no other beneficiaries will receive funds.
Impact High: Funds will be Loss, Intended distribution logic is broken, meaning rightful recipients do not receive their share.
Proof of Concept:
PoC
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(user1);
im.inherit();
im.withdrawInheritedFunds(address(0));
usdc.mint(address(user1), 10e6);
usdc.approve(address(im), type(uint256).max);
im.buyOutEstateNFT(1);
vm.stopPrank();
}
Recommended Mitigation:
To fix this issue, remove the return statement inside the loop and adjust the precision calculation:
function buyOutEstateNFT(
uint256 _nftID
) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
// ✅ FIX: Multiply before dividing to avoid precision loss
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 {
+ if (msg.sender != beneficiaries[i]) {
IERC20(assetToPay).safeTransfer(
beneficiaries[i],
finalAmount / divisor
);
}
}
nft.burnEstate(_nftID);
}
