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

assetToPay Override Leads to Price Manipulation of Existing NFTs

zebra

Summary

The assetToPay variable is globally defined but can be overridden when a new NFT asset is created. If an initial NFT asset (asset1) is created using WETH as payment, but a subsequent NFT (asset2) is created using USDC, the beneficiary can exploit the system by using a significantly higher USDC price to pay for asset1.

Impact: High

Likelihood: Low

Vulnerability Details

Each time createEstateNFT is called, it updates assetToPay, impacting all previously created NFT assets. If the previous asset used for payments differs from the newly assigned one, the required payment amount can become unreasonably high or low.

But, beneficiary can set trustee via appointTrustee(), and the trustee has the ability to set the nftValue and assetToPay. This will the solve the problem.

Impact

The only problem is if two beneficiary accounts are controlled by two different people. This creates the potential for a race condition.

If assetToPay changes from a lower-value asset (e.g., WETH) to a much higher-value one (e.g., WBTC), user has to overpay significantly to buy out the NFT tokens.

PoC

//SPDX-License-Identifier: MIT
​
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 NonReentrantTest is Test {
InheritanceManager im;
ERC20Mock usdc;
ERC20Mock weth;
​
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
vm.stopPrank();
usdc = new ERC20Mock();
weth = new ERC20Mock();
}
​
function test_buyOutEstateNFTWrongToken() public {
address user2 = makeAddr("user2");
vm.warp(1);
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.createEstateNFT("house1", 3e6, address(usdc));
im.createEstateNFT("house2", 1 ether, address(weth));
vm.stopPrank();
​
weth.mint(user1, 1 ether);
assertEq(usdc.balanceOf(user1), 0);
vm.warp(1 + 90 days);
vm.startPrank(user1);
// 3e6 wei
weth.approve(address(im), 3e6);
im.inherit();
im.buyOutEstateNFT(1);
vm.stopPrank();
assertEq(usdc.balanceOf(user1), 0);
console.log("weth balance of user1", weth.balanceOf(user1));
}
}

Tools Used

Recommendations

Use a mapping to store assetToPay for each individual NFT instead of a global variable.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

global asset in NFT values

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.