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

buyPresent function allows an exploiter to burn tokens from any address and receive an NFT in return

Summary

The buyPresent function in SantasList.sol, intended for trading SantaTokens for an NFT, contains a critical flaw. It allows an exploiter to burn tokens from any address and receive an NFT in return, without the consent of the token holder. Additionally, the function does not align with the intended trade value as specified in the contest details.

Vulnerability Details

The vulnerability arises from how the buyPresent function is implemented. It accepts an address presentReceiver parameter and burns SantaTokens from this address, but the NFT is minted to the caller of the function, not necessarily to presentReceiver. This discrepancy allows an exploiter to target any address holding SantaTokens, burn their tokens, and receive the corresponding NFT, effectively stealing the NFT without trading their own tokens. Furthermore, the function burns 1e18 SantaTokens (from SantasToken's burn function), which is inconsistent with the intended trade value of 2e18 as outlined in the contest details.

Impact

This vulnerability has severe implications:

  1. It allows unauthorized burning of SantaTokens from any address without their consent.

  2. It enables exploiters to unjustly acquire NFTs, thereby compromising the integrity of the token exchange system.

Tools Used

Manual Code Review

Recommendations

To address these vulnerabilities, the following changes are recommended:

  • Modify the buyPresent function to only allow token holders to trade their own SantaTokens for NFTs. This can be achieved by using msg.sender as the address for both burning tokens and minting NFTs.

  • Adjust the token burn quantity to align with the intended trade value of 2e18 SantaTokens.

Proposed update to the buyPresent function:

function buyPresent() external {
require(i_santaToken.balanceOf(msg.sender) >= 2e18, "Insufficient SantaTokens");
i_santaToken.burn(msg.sender);
_mintAndIncrement();
}

Proposed update to the burn function (SantaToken.sol):

function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_burn(from, 2e18);
}
Updates

Lead Judging Commences

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

buyPresent should use msg.sender

Current implementation allows a malicious actor to burn someone else's tokens as the burn function doesn't actually check for approvals.

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.