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

Estate Claim Collision After Estate is Bought Out

yeahchibyke

Summary

In the event that an estate is set up by the owner, any of the beneficiaries should be able to buy out the estate, with the proceeds then shared equally between the remaining beneficiaries.

Vulnerability Details

Logic flaws in the InheritanceManager::buyOutEstateNFT() function mean that all beneficiaries can buy out the estate at different points in time, and the respective transactions will go through.

Impact

This will lead to estate collision, as all beneficiaries can lay claim to the estate with an arguement that their bids(transactions) went through.

And through the test, we figure out that a dubious owner who is aware of this flaw can set up an estate, and steal a large amount of the funds generated from the sales of the estate.

Tools Used

  • Foundry

  • Manual Review

PoS

Consider this scenario:

  • Jim sets up the family beach house in Tuscany as an estate for his 3 beneficiaries -wife, son, and daughter- at a value of 150k USDC

  • Son thinks this is a good opportunity and buys out the estate at 100k USDC

  • Wife also buys out the estate at 100k USDC, unaware that her son has already exercised that option. The transaction goes through, so she doesn't any foul play

  • Daughter discusses with her fiance, and decides to buy out the estate. She goes ahead to pay the 100k USDC, unsuspectingly because the transaction goes through

  • The 3 beneficiaries get involved in a battle for who claims ownership of the house

  • Jim (or whoever is the owner) withdraws the USDC balance of the portfolio

PoC

Add the following test to the InheritanceManagerTest contract:

function test_poc() public {
address wife = makeAddr("wife");
address son = makeAddr("son");
address daughter = makeAddr("daughter");
​
vm.startPrank(owner);
im.createEstateNFT(" Beach house in Tuscany", 150_000e18, address(usdc)); // dubious owner sets up estate
im.addBeneficiery(wife);
im.addBeneficiery(son);
im.addBeneficiery(daughter);
vm.stopPrank();
​
vm.warp(1 + 90 days);
​
usdc.mint(son, 100_000e18);
vm.startPrank(son);
im.inherit();
usdc.approve(address(im), 100_000e18);
im.buyOutEstateNFT(1); //son buys out estate
vm.stopPrank();
​
uint256 portfolio = usdc.balanceOf(address(im));
assert(portfolio > 0);
​
usdc.mint(wife, 100_000e18);
vm.startPrank(wife);
usdc.approve(address(im), 100_000e18);
im.buyOutEstateNFT(1); // unaware wife buys out estate also
vm.stopPrank();
​
uint256 newPortfolio = usdc.balanceOf(address(im));
assert(newPortfolio > portfolio);
​
usdc.mint(daughter, 100_000e18);
vm.startPrank(daughter);
usdc.approve(address(im), 100_000e18);
im.buyOutEstateNFT(1); // unaware daughter also buys out estate
vm.stopPrank();
​
uint256 newNewPortfolio = usdc.balanceOf(address(im));
assert(newNewPortfolio > newPortfolio);
​
vm.prank(owner);
im.sendERC20(address(usdc), newNewPortfolio, owner); // dubious owner steals a large amount of sales gotten from estate sales
}

Recommendations

Make the following modifications to the ::buyOutEstateNFT() function:

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;
+ continue;
} else {
- IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
+ IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / multiplier);
}
}
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

yeahchibyke Submitter
5 months ago
0xtimefliez Lead Judge
5 months ago
0xtimefliez Lead Judge 5 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.