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

Owner can change cost of Treats if a user has been Tricked

delinked

Summary

The setTreatCost function allows the treat cost to be updated by the contract owner at any time. This can lead to unexpected behavior in scenarios where a user is resolving a "trick" transaction (double price) after trickOrTreat has been called. Specifically, if setTreatCost is called between trickOrTreat and resolveTrick, the required payment to resolve the trick will be modified unexpectedly, potentially resulting in failed transactions or unexpected cost for the user.

Vulnerability Details

The trickOrTreat function applies a double cost if the random value generated equals 2. This double-cost requirement is calculated at the time trickOrTreat is called, based on the treat's current cost. However, the owner can modify this cost with setTreatCost before the user completes their transaction with resolveTrick. As a result, the final payment required by resolveTrick becomes inconsistent with the amount anticipated by the user when they initiated trickOrTreat.

Proof of Code

The following test case demonstrates this inconsistency, where a treat named GoldenCandy initially has a cost of 1 ether. After calling trickOrTreat, the cost is increased to 3 ether before resolveTrick is called. As a result, the required payment is unexpectedly raised to 6 ether.

function testSetTreatCostDuringTrickOrTreat() public {
vm.prank(owner);
spookySwap.addTreat("GoldenCandy", 1 ether, "ipfs://Qm...GoldenCandy");
​
uint256 userBalanceBefore = user.balance;
​
// Set test mode to ensure the user gets tricked
vm.startPrank(owner);
spookySwap.setTestMode(true, 2); // Force double cost outcome
vm.stopPrank();
​
vm.startPrank(user);
spookySwap.trickOrTreat{value: 1 ether}("GoldenCandy");
vm.stopPrank();
​
// Owner changes the cost before the user can resolve the trick
vm.startPrank(owner);
spookySwap.setTreatCost("GoldenCandy", 3 ether);
vm.stopPrank();
​
// Log the updated treat cost for visibility
(string memory name, uint256 cost, string memory metadataURI) = spookySwap.treatList("GoldenCandy");
console.log("Updated treat cost after owner change:", cost);
​
// Expect revert since user balance should reflect the original cost only
vm.startPrank(user);
vm.expectRevert("Insufficient ETH sent to complete purchase");
spookySwap.resolveTrick{value: 1 ether}(1); // Token ID 1 for GoldenCandy
vm.stopPrank();
​
// Check that the user balance reflects only the initial deduction
uint256 userBalanceAfter = user.balance;
require(userBalanceAfter == userBalanceBefore - 1 ether, "User should only have the initial 1 ether deducted.");
}

Logs:

  • Updated treat cost after owner change: 3000000000000000000 (3 ether)

Impact

This vulnerability impacts the user experience by altering the expected transaction cost mid-process. Although no funds are directly at risk, the inconsistency in required payments can frustrate users and disrupt the expected behavior of the resolveTrick function.

Tools Used

Foundry testing framework, with console logs to track values during the test.

Recommendations

To prevent this vulnerability, modify the setTreatCost function to check for any pending "trick" transactions for the treat in question before allowing a cost update. Here is the modified code:

function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
require(treatList[_treatName].cost > 0, "Treat must cost something.");
​
// Loop through `pendingNFTs` to ensure no pending "trick" for the specified treat
for (uint256 i = 1; i < nextTokenId; i++) {
if (pendingNFTs[i] != address(0) && keccak256(abi.encodePacked(tokenIdToTreatName[i])) == keccak256(abi.encodePacked(_treatName))) {
revert("Cannot modify cost of treat with pending trick");
}
}
​
// If no pending trick, set the new cost
treatList[_treatName].cost = _cost;
}

Explanation

  • Loop and Check: We iterate through pendingNFTs to check if any token is pending with the treat name _treatName.

  • Revert on Pending Trick: If we find a match in the pending list, we revert with an error message.

  • Set Cost: If no pending trick is found, the cost is updated.

This adjustment prevents the cost from being changed while a trick is in process, maintaining consistency for users during the resolveTrick process.

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.