summary
When the owner would increase the price of an NFT, all the users that are in the pending state will also have to pay this new price if they want to receive their NFT without ever having consentet to the new price.
vulnerability details
the trickOrTreat does not safe the price of NFT at that point in time
if (costMultiplierNumerator == 2 && costMultiplierDenominator == 1) {
if (msg.value >= requiredCost) {
mintTreat(msg.sender, treat);
} 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;
emit Swapped(msg.sender, _treatName, tokenId);
}
}
impact
-
unfair cost to user: The user could be forced to pay more than they initially consented to or even be unable to complete the purchase if the new price exceeds their planned payment.
-
Potential for Denial of Service: Users who remain in a pending state might get "stuck" if the price increase renders it unfeasible for them to pay the new price, preventing them from obtaining the treat or completing the transaction.
-
Disruption in User Experience: This could lead to frustration and a poor user experience, as users might feel they are being forced into unfavorable terms post-transaction initiation.
note: decreasing the price would favor the user depending on how much the price gets decreased. But this is to the protocol to determin weather or not they want the pending users price to decrease to.
tools used
Manual review
recommendations
safe the price the user had to pay in a mapping for example AmountToPay:
if (costMultiplierNumerator == 2 && costMultiplierDenominator == 1) {
// Double price case (trick)
if (msg.value >= requiredCost) {
// User sent enough ETH
mintTreat(msg.sender, treat);
} else {
// User didn't send enough ETH
// Mint NFT to contract and store pending purchase
uint256 tokenId = nextTokenId;
_mint(address(this), tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
+ AmountToPay[tokenId] = requiredCost;
pendingNFTs[tokenId] = msg.sender;
pendingNFTsAmountPaid[tokenId] = msg.value;
tokenIdToTreatName[tokenId] = _treatName;
// do the pending nfts also come in the swapped event?
emit Swapped(msg.sender, _treatName, tokenId);
// User needs to call fellForTrick() to finish the transaction
}
}
and in resolveTrick:
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];
- uint256 requiredCost = treat.cost * 2; // Double price
+ uint256 requiredCost = AmountToPay[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];
// Refund excess ETH if any
if (totalPaid > requiredCost) {
uint256 refund = totalPaid - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}
}
```### summary
When the `owner` would increase the price of an NFT, all the users that are in the pending state will also have to pay this new price if they want to receive their NFT without ever having consentet to the new price.
### vulnerability details
the `trickOrTreat` does not safe the price of NFT at that point in time
```javascript
if (costMultiplierNumerator == 2 && costMultiplierDenominator == 1) {
// Double price case (trick)
if (msg.value >= requiredCost) {
// User sent enough ETH
mintTreat(msg.sender, treat);
} else {
// User didn't send enough ETH
// Mint NFT to contract and store pending purchase
uint256 tokenId = nextTokenId;
_mint(address(this), tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
@> pendingNFTs[tokenId] = msg.sender;
@> pendingNFTsAmountPaid[tokenId] = msg.value;
@> tokenIdToTreatName[tokenId] = _treatName;
// do the pending nfts also come in the swapped event?
emit Swapped(msg.sender, _treatName, tokenId);
// User needs to call fellForTrick() to finish the transaction
}
}
impact
-
unfair cost to user: The user could be forced to pay more than they initially consented to or even be unable to complete the purchase if the new price exceeds their planned payment.
-
Potential for Denial of Service: Users who remain in a pending state might get "stuck" if the price increase renders it unfeasible for them to pay the new price, preventing them from obtaining the treat or completing the transaction.
-
Disruption in User Experience: This could lead to frustration and a poor user experience, as users might feel they are being forced into unfavorable terms post-transaction initiation.
note: decreasing the price would favor the user depending on how much the price gets decreased. But this is to the protocol to determin weather or not they want the pending users price to decrease to.
tools used
Manual review
recommendations
safe the price the user had to pay in a mapping for example AmountToPay:
if (costMultiplierNumerator == 2 && costMultiplierDenominator == 1) {
// Double price case (trick)
if (msg.value >= requiredCost) {
// User sent enough ETH
mintTreat(msg.sender, treat);
} else {
// User didn't send enough ETH
// Mint NFT to contract and store pending purchase
uint256 tokenId = nextTokenId;
_mint(address(this), tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
+ AmountToPay[tokenId] = requiredCost;
pendingNFTs[tokenId] = msg.sender;
pendingNFTsAmountPaid[tokenId] = msg.value;
tokenIdToTreatName[tokenId] = _treatName;
// do the pending nfts also come in the swapped event?
emit Swapped(msg.sender, _treatName, tokenId);
// User needs to call fellForTrick() to finish the transaction
}
}
and in resolveTrick:
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];
- uint256 requiredCost = treat.cost * 2; // Double price
+ uint256 requiredCost = AmountToPay[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];
// Refund excess ETH if any
if (totalPaid > requiredCost) {
uint256 refund = totalPaid - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}
}
