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

The setNftValue function allows overwriting existing NFT values

0xWithdrawers

Description

The setNftValue() function in the Trustee contract allows associating a value with an NFT index. However, this function does not verify if a value already exists for the specified index. As a result, it is possible for the trustee to overwrite a previously defined value for an NFT, which might not be the expected behavior.

function setNftValue(uint256 _index, uint256 _value) public onlyTrustee {
nftValue[_index] = _value;
}

If the expected behavior was that NFT values should be immutable after their initial definition, then this functionality represents a vulnerability.

Impact

  1. Financial exploitation: In the buyOutEstateNFT function, beneficiaries can purchase NFTs at the value set by the nftValue mapping. If a trustee can arbitrarily change these values after they're set, they could:

    • Decrease the value of an NFT just before a beneficiary executes buyOutEstateNFT, causing other beneficiaries to receive less compensation

    • Manipulate values to favor certain beneficiaries over others

    • Significantly devalue assets that may represent substantial real-world value (e.g., real estate)

  2. Trust violation: The contract is explicitly designed for inheritance management where values should represent real-world assets. The ability to arbitrarily change these values undermines the entire trust model of the contract.

  3. Inconsistency with contract semantics: The createEstateNFT function's intended behavior is to set a one-time value for an NFT representing real estate or other high-value assets. The ability to change this value later contradicts the function's purpose and documentation.

  4. Collusion risk: A trustee could collude with one beneficiary to manipulate asset values in their favor, especially after the original owner becomes inactive.

Proof of Concept (PoC)

The following code demonstrates how an existing NFT value can be overwritten:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import "forge-std/Test.sol";
import "../src/modules/Trustee.sol";
​
// This PoC demonstrates that passing an index for an existing NFT overwrites the previous value
​
// Concrete implementation of the abstract Trustee contract
contract TrusteeImpl is Trustee {
constructor(address _trustee) {
trustee = _trustee;
}
}
​
contract OverwriteValuePoCTest is Test {
TrusteeImpl public trusteeContract;
address trusteeAddress = address(0x1);
function setUp() public {
// Deploy the contract with specified trustee
vm.prank(trusteeAddress);
trusteeContract = new TrusteeImpl(trusteeAddress);
}
function test_NftValueOverwrite() public {
uint256 nftIndex = 0;
// Display initial value (should be 0)
uint256 initialValue = trusteeContract.getNftValue(nftIndex);
console.log("Initial value of nftValue[0]:", initialValue);
// Trustee assigns a first value to index 0
vm.prank(trusteeAddress);
uint256 firstValue = 100;
trusteeContract.setNftValue(nftIndex, firstValue);
// Display the new value
uint256 valueAfterFirstSet = trusteeContract.getNftValue(nftIndex);
console.log("Value after first assignment (100):", valueAfterFirstSet);
// Trustee assigns a new value to index 0, overwriting the previous one
vm.prank(trusteeAddress);
uint256 secondValue = 200;
console.log("Trustee assigning a new value (200) to the same index");
trusteeContract.setNftValue(nftIndex, secondValue);
// Display the overwritten value
uint256 valueAfterSecondSet = trusteeContract.getNftValue(nftIndex);
console.log("Value after second assignment:", valueAfterSecondSet);
// Verify that the value has been successfully overwritten
assertEq(valueAfterSecondSet, secondValue, "Value was not correctly overwritten");
assertTrue(valueAfterSecondSet != firstValue, "Value did not change");
console.log("----- CONCLUSION -----");
console.log("The setNftValue function overwrites existing values for the same index");
}
}

Place the test in the test folder and run it with the following command

forge test --match-test test_nftValueOverwrite -vv

Recommendation

Based on the context of the InheritanceManager contract and how the NFT values are used to determine financial compensation during the buyout process, values should be immutable after their initial definition. We recommend modifying the setNftValue() function to check if a value already exists before allowing modification:

function setNftValue(uint256 _index, uint256 _value) public onlyTrustee {
require(nftValue[_index] == 0, "Value already set for this NFT index");
nftValue[_index] = _value;
}

Concrete Impact Example

To illustrate the potential financial impact of this vulnerability, consider this scenario:

  • Three beneficiaries (A, B, C)

  • NFT represents real estate valued at 900 USDC

  • Beneficiary A wants to buy the NFT

With the original value (900 USDC):

  • Payment calculation: (900 / 3) * 2 = 600 USDC total

  • Each remaining beneficiary receives: 600 / 2 = 300 USDC

If a trustee maliciously reduces the value to 300 USDC before the purchase:

  • Payment calculation: (300 / 3) * 2 = 200 USDC total

  • Each remaining beneficiary receives: 200 / 2 = 100 USDC

Updates

Lead Judging Commences

0xtimefliez Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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