Beatland Festival

First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

[M-3] Unrestricted token burn in `FestivalPass::redeemMemorabilia`


Description

In the `FestivalPass` contract, the `redeemMemorabilia` function allows users to redeem memorabilia NFTs
by burning BEAT tokens via:
​
```javascript
BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
```
However, the `burnFrom` function in BeatToken does not implement standard ERC-20 allowance checks.
It simply allows the festivalContract to burn any amount from any user unconditionally:
​
```javascript
function burnFrom(address from, uint256 amount) external {
require(msg.sender == festivalContract, "Only_Festival_Burn");
_burn(from, amount); // No allowance check
}
```
This effectively gives the FestivalPass contract unrestricted power to burn tokens from any user,
without their approval , as long as they call a function like redeemMemorabilia.
​

Risk

Impact:

1. Users cannot revoke permission to burn tokens.
​
2. No allowance pattern means loss of control over token spending.
​

Proof of Concept

Since burnFrom() lacks allowance checks, the contract can burn tokens without approval.
​
Put following code in `FestivalPass.t.sol` file
​
```javascript
function testRedeemMemorabiliaCanBurnMore() public {
// Setup collection
vm.prank(organizer);
uint256 collection = festivalPass.createMemorabiliaCollection("Test", "ipfs://", 1e18, 10, true);
​
// Give extra BEAT to user
vm.startPrank(address(festivalPass));
beatToken.mint(user1, 100e18);
vm.stopPrank();
​
// Balance before
uint256 before = beatToken.balanceOf(user1);
​
// Trigger redeem which burns price
vm.prank(user1);
festivalPass.redeemMemorabilia(collection);
​
// Check that only price was burned
assertEq(beatToken.balanceOf(user1), before - 1e18);
​
// Then trick: manually call burnFrom
vm.prank(address(festivalPass));
beatToken.burnFrom(user1, 50e18);
​
// Balance reduced further
assertEq(beatToken.balanceOf(user1), before - 51e18);
}
```
​
1. A malicious FestivalPass contract calls burnFrom(msg.sender, priceInBeat) with a much larger amount than expected.
​
2. The user has no way to restrict this burn because the burnFrom implementation lacks access control beyond the caller being the festivalContract.
​
Even if it's currently only called in redeemMemorabilia, a future code change or bug could lead to arbitrary burns.
​
​

Recommended Mitigation

Option 1 : Add a pre-check in FestivalPass to ensure the user has sufficient balance before burning:
​
```diff
// Redeem a memorabilia NFT from a collection
function redeemMemorabilia(uint256 collectionId) external {
MemorabiliaCollection storage collection = collections[collectionId];
require(collection.priceInBeat > 0, "Collection does not exist");
require(collection.isActive, "Collection not active");
require(collection.currentItemId < collection.maxSupply, "Collection sold out");
+ require(BeatToken(beatToken).allowance(msg.sender, address(this)) >= collection.priceInBeat, "Insufficient allowance");
BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
```
And/or refactor burnFrom in BeatToken to require an approve() or internal allowance check.
​
Additionally, you could consider renaming the burnFrom() function in BeatToken to indicate it's only
callable by the festival contract (e.g., _festivalBurn()),
to prevent confusion with the ERC-20 standard burnFrom() which expects allowance behavior.
​
Option 2 (Safer / Recommended): Use ERC-20 approve + transferFrom pattern with proper allowances.
​
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year 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!