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

Inconsistent Purchased Present Cost and Magic Value

Summary

There is an inconsistency in the purchased present cost between the variable, documentation, and the burn and mint functions. This inconsistency can lead to confusion and potential errors when calculating the cost of purchasing presents or performing token operations.

Vulnerability Details

The inconsistency in the purchased present cost can be observed in the following code snippet:

////// In SantasList.sol, the following variable is not use.
// The cost of santa tokens for naughty people to buy presents
uint256 public constant PURCHASED_PRESENT_COST = 2e18;
////// In SantaToken.sol
function mint(address to) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_mint(to, 1e18);
}
function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_burn(from, 1e18);
}
- `buyPresent`: A function that trades `2e18` of `SantaToken` for an NFT. This function can be called by anyone.

The PURCHASED_PRESENT_COST constant is defined as 2e18, indicating the cost of purchasing presents. However, both the mint and burn functions use 1e18 as the amount for minting and burning tokens. This inconsistency can lead to confusion and potential errors when calculating the cost of purchasing presents or performing token operations.

Impact

The inconsistency in the purchased present cost can have the following impacts:

  • Confusion in cost calculation: Developers or users relying on the documentation or variable definition may expect the cost of purchasing presents to be 2e18. However, the actual token operations use 1e18 as the amount, leading to confusion and potential miscalculations.

The impact is Low because the same amount is present in mint()  and burn(), so the number of present which can be booked by one user won’t change.

Tools Used

Manual review

Recommendations

To address the inconsistency in the purchased present cost, it is recommended to ensure consistency across all relevant components. This can be achieved by updating the mint and burn functions to use the PURCHASED_PRESENT_COST constant as the amount for minting and burning tokens. Additionally, the documentation and variable definition should be updated to reflect the correct value of 2e18 as the cost of purchasing presents.

// The cost of santa tokens for naughty people to buy presents
uint256 public constant PURCHASED_PRESENT_COST = 2e18;
function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
if (
s_theListCheckedOnce[msg.sender] == Status.NICE &&
s_theListCheckedTwice[msg.sender] == Status.NICE
) {
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE &&
s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender, PURCHASED_PRESENT_COST);
return;
}
revert SantasList__NotNice();
}
function buyPresent(address presentReceiver) external {
i_santaToken.burn(presentReceiver, PURCHASED_PRESENT_COST);
_mintAndIncrement();
}
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);
}

By ensuring consistency in the purchased present cost, it will help avoid confusion and potential errors when calculating the cost of purchasing presents or performing token operations.

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.