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

Inconsistent NFT Pricing Risks Funding Shortfall for the Protocol

Summary

NFT minting effectively costs 1e18 of ERC20 tokens for the user instead of documented 2e18, which leads to 50% loss for the protocol.

Vulnerability Details

SantasList::PURCHASED_PRESENT_COST is expected to be 2e18 tokens,

uint256 public constant PURCHASED_PRESENT_COST = 2e18;

This constant wasn't used anywhere in buyPresent() function

function buyPresent(address presentReceiver) external {
@> i_santaToken.burn(presentReceiver) ;
_mintAndIncrement();
}

coming to SantaToken burning function, we see hardcoded value of 1e18 to burn

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

Impact

Burning 1e18 instead of 2e18 potentialy leads to 50% loss of the protocol revenue.

Tools Used

Manual review

Recommendations

Use PURCHASED_PRESENT_COST as an input for the burn() function inside buyPresent().

SantasList changes:

function buyPresent(address presentReceiver) external {
- i_santaToken.burn(presentReceiver) ;
+ i_santaToken.burn(presentReceiver, PURCHASED_PRESENT_COST) ;
_mintAndIncrement();
}

SantaToken changes:

function burn(address from, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
- _burn(from, 1e18);
+ _burn(from, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
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.