Beatland Festival
AI First Flight #4
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

# L-3: `BeatToken.burnFrom` has no allowance — festival contract holds unscoped burn power

rubabbel

Root + Impact

BeatToken.burnFrom(from, amount) destroys from's balance with only a msg.sender == festivalContract check and no ERC20 allowance. It is therefore not "burn from with permission" — it is "burn anyone's balance." It is safe today only because the sole caller, redeemMemorabilia, always passes msg.sender as from. Any change to the festival contract that passes a different from (or a compromised/upgraded festival contract) can burn any holder's tokens without consent.

Description

// src/BeatToken.sol:24-27
function burnFrom(address from, uint256 amount) external {
@> require(msg.sender == festivalContract, "Only_Festival_Burn");
@> _burn(from, amount); // no allowance(from, msg.sender) check
}

The name and signature imply an ERC20Burnable.burnFrom that checks allowance, but the implementation bypasses allowance entirely. The current caller is correct:

// src/FestivalPass.sol:197
BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);

Risk

Likelihood: Low. There is no current code path that passes a third-party from, so the bug is latent. It becomes live the moment a new festival function redeems on behalf of a user, or the festival contract is replaced/compromised.

Impact: If triggered, arbitrary holders' BEAT can be burned, destroying value with no user approval. Latent today; flag it so the invariant is not accidentally violated by future code.

Proof of Concept

The test deploys a MockFestival and makes it the festivalContract. It then burns a victim's balance with no approval from the victim, proving the callee performs no allowance check. (Marked latent: in production the festival contract is trusted and currently only burns msg.sender.)

// test/Exploits.t.sol (add)
contract MockFestival {
BeatToken bt;
constructor(BeatToken _bt) { bt = _bt; }
function burnVictim(address victim, uint256 amt) external { bt.burnFrom(victim, amt); }
}
​
function test_BurnFromIgnoresAllowance() public {
BeatToken t = new BeatToken();
MockFestival mf = new MockFestival(t);
t.setFestivalContract(address(mf)); // owner = this test
​
address victim = makeAddr("victim");
vm.prank(address(mf));
t.mint(victim, 100e18);
​
// no approval ever granted by victim
assertEq(t.allowance(victim, address(mf)), 0);
mf.burnVictim(victim, 100e18); // succeeds anyway
assertEq(t.balanceOf(victim), 0); // bug proof: balance burned without allowance
}

Recommended Mitigation

Implement the conventional allowance-preserving burn, or have the festival contract burn from msg.sender directly. If on-behalf-of burning is intended, require from == msg.sender or an explicit allowance.

function burnFrom(address from, uint256 amount) external {
require(msg.sender == festivalContract, "Only_Festival_Burn");
+ if (from != msg.sender) {
+ _spendAllowance(from, msg.sender, amount);
+ }
_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!