Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

Users can buy present at a lower cost because `SantasList::buyPresent(address)` function don't burn `SantasList::PURCHASED_PRESENT_COST` amount of `SantaToken`

Summary

Users can buy present at a lower cost because SantasList::buyPresent(address) function don't burn SantasList::PURCHASED_PRESENT_COST amount of SantaToken.

Vulnerability Details

Any user can buy a present (NFT) spending only 1e18 Santa Tokens instead of the specified cost of 2e18 - the PURCHASED_PRESENT_COST constant - through the SantasList::buyPresent(address) function.

Proof of Concept

Apply the following diff:

modified test/unit/SantasListTest.t.sol
@@ -152,4 +152,48 @@ contract SantasListTest is Test {
cmds[1] = string.concat("youve-been-pwned");
cheatCodes.ffi(cmds);
}
+
+ function testIncorrectPurchasedPresentCost() public {
+ // Impersonating Santa
+ vm.startPrank(santa);
+
+ // Check `user` for EXTRA_NICE status
+ santasList.checkList(user, SantasList.Status.EXTRA_NICE);
+ santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
+
+ // Stops impersonating Santa
+ vm.stopPrank();
+
+ // Christmas Time!
+ vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
+
+ // Present price
+ uint256 presentPrice = santasList.PURCHASED_PRESENT_COST();
+
+ // Impersonates `user`
+ vm.startPrank(user);
+
+ // Approve Santas List
+ santaToken.approve(address(santasList), presentPrice);
+
+ // Collect NFT and Santa Tokens
+ santasList.collectPresent();
+
+ // Santa Tokens balance before buying present
+ uint256 balanceBefore = santaToken.balanceOf(user);
+
+ // Buy a present using 1e18 Santa Tokens
+ santasList.buyPresent(user);
+
+ // Santa Tokens balance after buying present
+ uint256 balanceAfter = santaToken.balanceOf(user);
+
+ // Stops impersonating `user`
+ vm.stopPrank();
+
+ // Assertions
+ assertGt(presentPrice, balanceBefore);
+ assertGt(balanceBefore, balanceAfter);
+ assertEq(santasList.balanceOf(user), 2);
+ }
}

Then run the testIncorrectPurchasedPresentCost test:

forge test --match-test testIncorrectPurchasedPresentCost

Impact

Users can buy the present for half of the specified price.

Tools Used

  • Manual Review

  • GNU Emacs (solidity-mode + magit)

  • Foundry test

Recommendations

Modify burn(address) function of the SantaToken contract to receive a uint256 argument to specify the amount of Santa Tokens to be burned:

modified src/SantaToken.sol
@@ -25,10 +25,10 @@ contract SantaToken is ERC20 {
_mint(to, 1e18);
}
- function burn(address from) external {
+ function burn(address from, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
- _burn(from, 1e18);
+ _burn(from, amount);
}
}

And modify the buyPresent(address) function of SantasList contract to pass the constant PURCHASED_PRESENT_COST as 2nd argument to the i_santaToken.burn(address,uint256) function:

modified src/SantasList.sol
@@ -170,7 +170,7 @@ contract SantasList is ERC721, TokenUri {
* @dev You'll first need to approve the SantasList contract to spend your SantaTokens.
*/
function buyPresent(address presentReceiver) external {
- i_santaToken.burn(presentReceiver);
+ i_santaToken.burn(presentReceiver, PURCHASED_PRESENT_COST);
_mintAndIncrement();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Price is not enforced in buyPresent

This line indicates that the intended cost of presents for naughty people should be 2e18: https://github.com/Cyfrin/2023-11-Santas-List/blob/6627a6387adab89ae2ba2e82b38296723261c08a/src/SantasList.sol#L87 PURCHASE_PRESENT_COST should be implemented to enforce the cost of presents.

Support

FAQs

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