Trick or Treat

Summary

In the SpookySwap smart contract, after a user is "tricked" by not sending enough ETH during a purchase, they are required to call the resolveTrick function to complete the transaction by paying the remaining amount. However, the contract allows the owner to change the price of the NFT (the treat) after the user has been tricked but before they resolve the trick. This means the owner can increase the required payment arbitrarily, forcing the user to pay more than the originally agreed-upon double price to receive the NFT. This vulnerability enables the owner to exploit users financially and undermines trust in the platform. The owner may not know that the NFT is already pending a purchase, and the price increase could be inadvertent.

Vulnerability Details

Issue Overview

• Function Involved: resolveTrick(uint256 tokenId)

• Vulnerability Type: Price Manipulation / Owner Privilege Abuse

• Root Cause: The required cost is recalculated at the time of resolveTrick, allowing the owner to change the treat's price between the initial purchase attempt and the resolution, affecting the user's final payment amount.

Affected Code

Problem Explanation

• Dynamic Price Calculation: The requiredCost is recalculated when resolveTrick is called:

  uint256 requiredCost = treat.cost * 2; // Double price

• Owner Privilege to Change Price: The owner can change the treat.cost at any time using the setTreatCost function.

• Vulnerability Exploitation: Between the user's initial attempt (where they didn't send enough ETH) and their call to resolveTrick, the owner can increase treat.cost, thus increasing requiredCost. The user is then forced to pay more than expected to obtain the NFT.

Proof of Concept (PoC)

Step-by-Step Exploitation:

1. User Attempts Purchase and Is Tricked:

   • User calls trickOrTreat but sends less than the required cost.

   • NFT is minted to the contract, and the user must call resolveTrick to complete the purchase.

2. Owner Increases the Treat Price:

   • Owner calls setTreatCost to increase the treat.cost for the specific treat.

3. User Calls resolveTrick:

   • User calls resolveTrick, expecting to pay the original double price.

   • Due to the increased treat.cost, requiredCost is now higher.

   • User must pay the new, higher amount to receive the NFT.

Example Code:

Paste this into a solidity test folder as a new test file. (eg. TestPriceChangeVuln.t.sol)

Explanation:

User is the Attacker in the PoC.

• Initial Setup:

  • The treat "Candy" costs 1 ether.

  • User has 10 ether.

• User Purchase Attempt:

  • User sends 1 ether, but a trick is initiated and the user needs to send 2 ether. User will need to call the resolveTrick function and send more ether to receive the NFT.

• Owner Price Increase:

  • Owner increases the treat cost to 4 ether.

  • Required cost in resolveTrick becomes 4 ether * 2 = 8 ether.

• User Tries to Resolve Trick:

  • User attempts to pay the remaining amount, expecting to pay a total of 2 ether (double the original cost).

  • Transaction reverts with "Insufficient ETH sent to complete purchase" because the required cost is now 8 ether.

Impact

• Financial Exploitation:

  • Users are forced to pay more than the agreed-upon price to obtain the NFT.

  • The owner can effectively hold the NFT hostage until the user pays the increased amount.

• Trust Erosion:

  • Users lose trust in the platform due to unfair practices.

  • Potential legal and reputational repercussions for manipulating prices after a transaction has been initiated.

• Violation of Expectations:

  • Users expect the price to be fixed at the time of purchase.

  • Changing the price retroactively breaches the implicit agreement.

Severity Classification: Medium

• Likelihood: Low to Medium

  • The owner must intentionally manipulate the price after a user has been tricked.

  • Such behavior is likely to be noticed and could deter users from the platform.

• Impact: High for Affected Users

  • Users may suffer financial loss if they choose to pay the higher price.

  • Alternatively, they may lose the opportunity to obtain the NFT they intended to purchase.

Tools Used

Manual Review

Recommendations

1. Lock the Price at the Time of Initial Purchase Attempt

• Store the Required Cost:

  Modify the contract to store the requiredCost at the time the user is tricked.

  Code Changes:

  // Inside the else block where the user didn't send enough ETH

  else {

  uint256 tokenId = nextTokenId;

  _mint(address(this), tokenId);

  _setTokenURI(tokenId, treat.metadataURI);

  nextTokenId += 1;

  ​

  pendingNFTs[tokenId] = msg.sender;

  pendingNFTsAmountPaid[tokenId] = msg.value;

  tokenIdToTreatName[tokenId] = _treatName;

  ​

  + // Store the required cost at the time of trick

  + pendingNFTsRequiredCost[tokenId] = treat.cost * 2; // Double price

  }

  Data Structure Addition:

  mapping(uint256 => uint256) public pendingNFTsRequiredCost;

• Use the Stored Cost in resolveTrick:

  Modify resolveTrick to use the stored requiredCost instead of recalculating it.

  Code Changes:

  function resolveTrick(uint256 tokenId) public payable nonReentrant {

  require(pendingNFTs[tokenId] == msg.sender, "Not authorized to complete purchase");

  ​

  string memory treatName = tokenIdToTreatName[tokenId];

  // Treat memory treat = treatList[treatName]; // No longer needed

  ​

  - uint256 requiredCost = treat.cost * 2; // Double price

  + uint256 requiredCost = pendingNFTsRequiredCost[tokenId];

  ​

  uint256 amountPaid = pendingNFTsAmountPaid[tokenId];

  uint256 totalPaid = amountPaid + msg.value;

  ​

  require(totalPaid >= requiredCost, "Insufficient ETH sent to complete purchase");

  ​

  // Transfer the NFT to the buyer

  _transfer(address(this), msg.sender, tokenId);

  ​

  // Clean up storage

  delete pendingNFTs[tokenId];

  delete pendingNFTsAmountPaid[tokenId];

  delete tokenIdToTreatName[tokenId];

  + delete pendingNFTsRequiredCost[tokenId];

  ​

  // Refund excess ETH if any

  // ... existing code ...

  }

Benefits:

• Price Stability: Ensures users pay the expected amount, maintaining trust.

• Prevents Exploitation: Owner cannot manipulate the price after the initial purchase attempt.

Final Assessment: The vulnerability is classified as Medium severity due to the potential for significant user impact, even though the likelihood is reduced by the need for intentional/accidental owner misconduct.