Trick or Treat
First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Owner of the contract can manipulate the price of the nft for users when trying to call resolveTrick function

udo

Summary

A critical price manipulation vulnerability has been identified in the SpookyTreats NFT contract. The vulnerability allows the contract owner to maliciously manipulate the price of an NFT after a user has initiated a purchase on the trickOrTreat() before they complete it, forcing the user to pay significantly more than the original price to receive their NFT.

Vulnerability Details

The vulnerability exists in the interaction between trickOrTreat() and resolveTrick() functions, specifically when a "trick" scenario occurs. The issue arises because:

  1. When a user gets "tricked" (1/1000 chance), they initially pay a portion of the total cost

  2. The NFT is minted to the contract and marked as pending

  3. The contract stores:

    • The pending NFT owner (pendingNFTs mapping)

    • The amount already paid (pendingNFTsAmountPaid mapping)

    • The treat name (tokenIdToTreatName mapping)

  4. However, the contract does not store the original price of the treat

  5. When resolveTrick() is called, it calculates the required cost using the current price: uint256 requiredCost = treat.cost * 2; // Double price

  6. The owner can call setTreatCost() to change the price between the initial purchase and resolution:

    function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
    require(treatList[_treatName].cost > 0, "Treat must cost something.");
    treatList[_treatName].cost = _cost;
    }

Impact

The vulnerability has severe implications:

  1. Financial Loss: Users can be forced to pay significantly more than the advertised price

  2. Trapped Funds: Users who cannot afford the manipulated price lose their initial payment

Tools Used

Manual code review

Proof of Concept

Proof of Concept code demonstrates that:

  • Initial treat cost: 0.1 ETH

  • User pays: 0.12 ETH initially

  • Owner increases price to 1 ETH

  • User must pay an additional 1.88 ETH (total 2 ETH) to receive their NFT

  • Result: User pays 20x more than the original price

The following Foundry test will show that the owner can manipulate the price of the NFT

function testPriceManipulationVulnerability() public {
// Add Treat
spookySwap.addTreat("Cookie", 0.1 ether, "ipfs://QmExample");
uint256 INITIAL_COST = 0.1 ether;
uint256 initialPayment = 0.12 ether;
uint256 MANIPULATED_COST = 1 ether;
// Give user some ETH
vm.deal(user, 10 ether);
// Manipulate random number to be 2 (double price)
uint256 timestamp = 708;
uint256 tokenId = 1;
uint256 prevrandao = 708;
vm.warp(timestamp);
vm.prevrandao(bytes32(prevrandao));
uint256 expectedRandom = calculateRandom(
timestamp,
user,
tokenId,
prevrandao
);
uint256 targetTokenId = spookySwap.nextTokenId();
console.log("targetTokenId", targetTokenId);
console.log("Calculated Random Value:", expectedRandom);
bytes32 predictedHash = keccak256(abi.encodePacked(
block.timestamp,
user,
targetTokenId,
block.prevrandao
));
​
uint256 initialBalance = user.balance;
console.log("initialBalance",initialBalance);
​
vm.prank(user);
spookySwap.trickOrTreat{value: initialPayment}("Cookie");
uint256 balanceAfterTrickorTreat = user.balance;
console.log("balanceAfterTrickorTreat",balanceAfterTrickorTreat);
// Verify NFT is minted to contract and pending purchase exists
// Verify NFT was minted to contract
assertEq(spookySwap.ownerOf(tokenId), address(spookySwap));
// Verify payment was recorded
assertEq(spookySwap.pendingNFTsAmountPaid(tokenId), initialPayment);
// Verify correct user is recorded
assertEq(spookySwap.pendingNFTs(tokenId), user);
// Calculate how much more the user needs to pay before manipulation
uint256 requiredAdditionalPaymentBeforeManipulation = (INITIAL_COST * 2) - initialPayment;
​
spookySwap.setTreatCost("Cookie", MANIPULATED_COST);
// User tries to resolve the trick with the actual remaining payment but reverts because the price of the nft has been manipulated
vm.expectRevert("Insufficient ETH sent to complete purchase");
vm.prank(user);
spookySwap.resolveTrick{value: requiredAdditionalPaymentBeforeManipulation}(tokenId);
​
// Calculate how much more the user needs to pay after manipulation
uint256 requiredAdditionalPaymentAfterManipulation = (MANIPULATED_COST * 2) - initialPayment;
uint256 balanceBeforerResolveTrick = user.balance;
console.log("balanceBeforerResolveTrick",balanceBeforerResolveTrick);
​
vm.prank(user);
spookySwap.resolveTrick{value: requiredAdditionalPaymentAfterManipulation}(tokenId);
​
uint256 balanceAfterResolveTrick = user.balance;
console.log("balanceAfterResolveTrick",balanceAfterResolveTrick);
// Verify user now owns the NFT but paid much more than they should have
assertEq(spookySwap.ownerOf(tokenId), user);
// Calculate total paid by user
uint256 totalPaid = initialPayment + requiredAdditionalPaymentAfterManipulation;
console.log("totalPaid",totalPaid);
// Verify user paid based on manipulated price
assertEq(totalPaid, MANIPULATED_COST * 2);
}

Recommendations

Store the Original Price of the NFT

struct PendingPurchase {
address buyer;
uint256 amountPaid;
uint256 originalPrice;
string treatName;
}
​
mapping(uint256 => PendingPurchase) public pendingPurchases;
​
function trickOrTreat(string memory _treatName) public payable {
// ... existing code ...
pendingPurchases[tokenId] = PendingPurchase({
buyer: msg.sender,
amountPaid: msg.value,
originalPrice: treat.cost,
treatName: _treatName
});
}
​
function resolveTrick(uint256 tokenId) public payable {
PendingPurchase memory purchase = pendingPurchases[tokenId];
uint256 requiredCost = purchase.originalPrice * 2;
// ... rest of the function ...
}
​
Updates

Appeal created

bube Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[invalid] Change cost between the call of trickOrTreat and resolveTrick

Only the owner has the rights to change the cost of the treat. Therefore it is assumed that the owner will not change the cost of the pending NFTs. The owner role is trusted.

Support

FAQs

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