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

`buyPresent` function currently trades `1e18` SantaToken for an NFT while documentation says it should be traded at `2e18` tokens.

tadev

Summary

Documentation says: "buyPresent: A function that trades 2e18 of SantaToken for an NFT. This function can be called by anyone."
With the current implementation, buyPresent function in SantasList contract i_santaToken.burn function. This function actually burns 1e18 token, and after its execution, the new NFT is minted

Moreover, the present cost is set in a constant variable :

uint256 public constant PURCHASED_PRESENT_COST = 2e18;

But this variable is never used in the contract.

There is another issue related to the cost of presents : any EXTRA_NICE user will receive 1e18 SantaToken when calling collectPresent function. Once this functions has been called and a NFT has been minted on the user's address, it will not be possible to call it again to receive another 1e18 token, finally reaching 2e18 tokens. Indeed, this checks :

if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}

makes sure that any user who already minted an NFT will not be able to collect present again.
Actually, if the user calls transfer function to send the NFT to another address, it will be possible to call collectPresent function again but this finding is out of the scope of this issue.

Vulnerability Details

Impact

The impact of this issue is MEDIUM as it allows anyone allowed to call buyPresent to mint an NFT for half of the supposed price.

Tools Used

Manual

Recommendations

Different approach are valid to solve this issue.
SantaToken contract could be modified to mint and burn exactly 2e18 tokens instead of 1e18. This way, EXTRA_NICE people will get 2e18 tokens when calling collectPresent function, and calls to buyPresent function will be updated to 2e18, which is the correct purchase cost.

It would also be possible to not hardcode mint and burn amount in SantaToken contract, as follows :

function mint(address to, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_mint(to, amount);
}
​
function burn(address from, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_burn(from, amount);
}

This way, SantasList contract could use the declared constant variable PURCHASED_PRESENT_COST in buyPresent and collectPresent functions:

// mint PURCHASED_PRESENT_COST (or more if the protocol decides an EXTRA_NICE user can get more)
i_santaToken.mint(msg.sender, PURCHASED_PRESENT_COST);
function buyPresent(address presentReceiver) external {
// This will set the price of an NFT to 2e18 tokens
i_santaToken.burn(presentReceiver, PURCHASED_PRESENT_COST );
_mintAndIncrement();
}

Note that buyPresent function's logic is incorrect, but out of the scope of this finding.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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