Summary
The InheritanceManager::buyOutEstateNFT function contains an error where the function prematurely exits when the buyer address is next in the beneficiaries loop. This occurs due to the return statement used inside the loop. As a result, the buyer does not pay the required amount to the remaining beneficiaries that are after the buyer in the array.
Impact
Loss of funds for other beneficiaries.
POC
Place the following test in /test/InheritanceManagerTest.t.sol:
function test_unpaid_nft() external {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.warp(1);
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.createEstateNFT("our beach-house", 3e6, address(usdc));
vm.stopPrank();
vm.warp(1 + 90 days);
uint256 user1InitialBalance = 4e6;
usdc.mint(user1, user1InitialBalance);
assertEq(usdc.balanceOf(user2), 0);
assertEq(usdc.balanceOf(user3), 0);
uint256 expectedShare = im.getNftValue(1) / 2;
uint256 user1FinalBalance = (im.getNftValue(1) / 3) * 2;
vm.startPrank(user1);
usdc.approve(address(im), 4e6);
im.inherit();
im.buyOutEstateNFT(1);
vm.stopPrank();
assertNotEq(usdc.balanceOf(user2), expectedShare);
assertNotEq(usdc.balanceOf(user3), expectedShare);
assertEq(usdc.balanceOf(user2), 0);
assertEq(usdc.balanceOf(user3), 0);
}
Tools Used
Recommendations
Use the continue statement instead of the return statement. Or put the transfer in an if block.
Using an if statement:
function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
- uint256 finalAmount = (value / divisor) * multiplier;
+ uint256 finalAmount = (value * multiplier) / divisor;
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);
- }
+ if (msg.sender != beneficiaries[i]) {
+ IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / multiplier);
+ }
}
nft.burnEstate(_nftID);
}
