First Flight #5: Santa's List
First Flight #5
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Cost to buy a present is `2e18` of `SantaToken` according to the docs. but, Anyone can buy a present in exchange of `1e18` of `SantaToken`.

naman1729

Summary

  • Cost to buy a present is 2e18 of SantaToken according to the documentation. but, Anyone can buy a present in exchange of 1e18 of SantaToken. because, buyPresent function of SantasList contract calls burn function of SantaToken contract with 1e18 instead of 2e18.

Vulnerability Details

  • Below test shows that if someone is extra nice and collect his present then he can buy a present in exchange of his 1e18 SantaTokens. But In the Documentation file, it is mentioned that to buy a present, one must burn his 2e18 of SantaTokens.

// this test show that if someone is extra nice and collect his present then he can buy a present in exchange of his 1e18 SantaTokens.
// But In the README.md(Documentation) file, it is mentioned that to buy a present, one must burn his 2e18 SantaTokens not 1e18 :
// `buyPresent`: A function that trades `2e18` of `SantaToken` for an NFT. This function can be called by anyone.
function testBuyPresentWhereSomeoneCanBuyNftIn_1e18_InsteadOf_2e18() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
santaToken.approve(address(santasList), 1e18);
santasList.collectPresent();
// These assert check that user has 1 SANTA(NFT) and 1e18 of SantaToken.
assertEq(santasList.balanceOf(user), 1);
assertEq(santaToken.balanceOf(user), 1e18);
santasList.buyPresent(user);
// These assert show that someone can buy a present in exchange of 1e18 of SantaToken instead of 2e18.
assertEq(santasList.balanceOf(user), 2);
assertEq(santaToken.balanceOf(user), 0);
vm.stopPrank();
}

  • This is the terminal output of the above test. It shows that the test is passed but it should be failed because someone can buy a present in exchange of 1e18 of SantaToken instead of 2e18.

Click to show terminal output
[⠒] Compiling...
[⠒] Compiling 1 files with 0.8.22
[⠆] Solc 0.8.22 finished in 2.90s
Compiler run successful!
​
Running 1 test for test/unit/SantasListTest.t.sol:SantasListTest
[PASS] testBuyPresentWhereSomeoneCanBuyNftIn_1e18_InsteadOf_2e18() (gas: 206870)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 22.20ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

  • This is the burn function of SantaToken contract.

Click to show burn() function
function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
@> _burn(from, 1e18);
}

Impact

  • Anyone can buy a present in exchange of 1e18 of SantaToken instead of 2e18 of SantaToken.

Tools Used

  • Manual Review

  • foundry

Recommendations

  • In the buyPresent function of SantasList contract, burn function of SantaToken contract should be called with PURCHASED_PRESENT_COST(2e18).

/*
* @notice Buy a present for someone else. This should only be callable by anyone with SantaTokens.
* @dev You'll first need to approve the SantasList contract to spend your SantaTokens.
*/
function buyPresent(address presentReceiver) external {
- i_santaToken.burn(presentReceiver);
- _mintAndIncrement();
+ i_santaToken.burn(msg.sender, PURCHASED_PRESENT_COST);
+ _safeMint(presentReceiver, s_tokenCounter++);
}

  • In the burn function of SantaToken contract, we add an argument amount in the function and also check that amount should not be zero.

+ error SantaToken__AmountIsZero();
function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_burn(from, 1e18);
}
+ function burn(address from, uint256 amount) external {
+ if (msg.sender != i_santasList) {
+ revert SantaToken__NotSantasList();
+ }
+ if (amount == 0){
+ revert SantaToken__AmountIsZero();
+ }
+
+ _burn(from, amount);
+ }
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.