Submission Details
Severity:
`InheritanceManager:buyOutEstateNFT` misplaced return can lead to unfair distribution
Summary
Misplaced return causes unfair distribution
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
@> return;
} else {
IERC20(assetToPay).safeTransfer(
beneficiaries[i],
finalAmount / divisor
);
}
}
Vulnerability Details
This return breaks the execution if the msg.sender is first on the list, and nobody else will receive their assets. Instead of that, the assets will be stored in the contract, and after that, an unfair beneficiary can call withdrawInheritedFunds and distribute part of their funds.
function test_UnfairNFTBuy() public {
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();
usdc.mint(user2, 4e6);
console.log("Balances before:");
console.log("User1:", usdc.balanceOf(user1));
console.log("User2:", usdc.balanceOf(user2));
console.log("User3:", usdc.balanceOf(user3));
console.log("Contract:", usdc.balanceOf(address(im)));
vm.warp(1 + 90 days);
vm.startPrank(user2);
usdc.approve(address(im), 4e6);
im.inherit();
im.buyOutEstateNFT(1);
im.withdrawInheritedFunds(address(usdc));
vm.stopPrank();
console.log("Balances after:");
console.log("User1:", usdc.balanceOf(user1));
console.log("User2:", usdc.balanceOf(user2));
console.log("User3:", usdc.balanceOf(user3));
console.log("Contract:", usdc.balanceOf(address(im)));
}
Balances before:
User1: 0
User2: 4000000
User3: 0
Contract: 0
Balances after:
User1: 1111110
User2: 2444444
User3: 444444
Contract: 2
It is not a problem if the caller is the first beneficiary, but it is an issue if the caller is different from the first or last
Recommendations
It should be continue instead
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
- return;
+ continue;
} else {
IERC20(assetToPay).safeTransfer(
beneficiaries[i],
finalAmount / divisor
);
}
}
Updates
Lead Judging Commences
0xtimefliez 4 months ago
Submission Judgement Published Assigned finding tags:
buyOutNFT has return instead of continue
Rewards breakdown
nSLOC
211This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.