Santa's List
AI First Flight #3
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

SantaToken.burn() should take amount parameter which is passed to _burn()

datadata

Root + Impact

Description

  • SantaToken.burn() does not take an amount variable.

  • Because SantaToken.burn() doesn't take an amount variable, it cannot then pass that amount to the _burn() function, which is defined to take an amount variable. Instead, burn() passes 1e18 as the amount to _burn()

// definition of SantaToken.burn()
@> function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
@> _burn(from, 1e18);
}
​
// definition of _burn() in solmate/src/tokens/ERC20.sol:
@> function _burn(address from, uint256 amount) internal virtual {
balanceOf[from] -= amount;
​
// Cannot underflow because a user's balance
// will never be larger than the total supply.
unchecked {
totalSupply -= amount;
}
​
emit Transfer(from, address(0), amount);
}

Risk

Likelihood:

  • This will happen anytime burn() is successfully called, which will always pass 1e18 as the amount.

Impact:

  • The impact is that the increased functionality of burning more than 1e18 of the token at a time is now lost.

  • Instead, if one wants to burn more than 1e18 of the token, they must repeatedly call burn() until their desired amount is burned.

Proof of Concept

This shows how only 1e18 is burned when burn() is called. We see here also an updated version of burn() that takes an amount variable, which then can pass that amount to _burn() for an easier method of buring tokens rather than repeated calls burning 1e18 at a time.

// Test function in SantasListTest.t.sol
// shows that burn() burns 1e18, the smallest amount for uint256
function testBurnBurnsSmallestAmount() public {
// ARRANGE
// fund or deal token to user3
uint256 balanceBefore = santaToken.balanceOf(user3);
​
// ACT
vm.startPrank(santa);
santaToken.burn(msg.sender);
vm.stopPrank();
​
// ASSERT
uint256 balanceAfter = santaToken.balanceOf(user3);
uint256 balanceDifference = balanceBefore - balanceAfter;
assertEq(balanceDifference, 1e18);
}
​
​
// newBurn is added to SantaToken
function newBurn(address from, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_burn(from, amount);
}
​
// Test function in SantasListTest.t.sol
// Shows that newBurn() allows for burning a specified amount via the parameter passed to it
function testNewBurnBurnsSpecifiedAmount() public {
// ARRANGE
// fund or deal token to user3
uint256 balanceBefore = santaToken.balanceOf(user3);
uint256 testAmount = 3;
​
​
// ACT
vm.startPrank(santa);
santaToken.newBurn(msg.sender, testAmount);
vm.stopPrank();
​
// ASSERT
uint256 balanceAfter = santaToken.balanceOf(user3);
uint256 balanceDifference = balanceBefore - balanceAfter;
assertEq(balanceDifference, testAmount);
}

Recommended Mitigation

We recommend rewriting the definition of burn() so that it can take a uint256 amount parameter that can be passed on to _burn().

- remove this code
​
​
+ add this code
+ function burn(address from, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
+ _burn(from, amount);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 4 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!