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

CEI Pattern Violation in redeemMemorabilia Function: Code Quality Issue

jamiebones147

Root + Impact

Description

The redeemMemorabilia function in the FestivalPass contract violates the Checks-Effects-Interactions (CEI) pattern by performing external calls before updating state variables. While this doesn't create an exploitable reentrancy vulnerability in the current implementation (since BeatToken uses standard ERC20 without hooks), it represents poor coding practices that could become problematic if the token contract is upgraded or replaced.

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");
​
// External call before state updates (violates CEI)
@> BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
​
// State changes happen AFTER external call
uint256 itemId = collection.currentItemId++;
uint256 tokenId = encodeTokenId(collectionId, itemId);
tokenIdToEdition[tokenId] = itemId;
​
_mint(msg.sender, tokenId, 1, "");
emit MemorabiliaRedeemed(msg.sender, tokenId, collectionId, itemId);
}

Risk

Likelihood:

  • This is a deviation from the accepted pattern

Impact:

  • Code quality and maintainability concerns

  • Deviation from security best practices

Proof of Concept

BeatToken is a standard ERC20 implementation using OpenZeppelin contracts

  • The burnFrom function only calls _burn(from, amount) with no hooks or callbacks

  • No reentrancy vector exists in the current token implementation

​

Recommended Mitigation

  • Consider adding a reentrancy guard using OpenZeppelin's ReentrancyGuard

  • Ensure all functions follow the CEI pattern consistently.

  • Add comprehensive testing for reentrancy scenarios

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");
- BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
// 2. EFFECTS - Update state BEFORE external calls
uint256 itemId = collection.currentItemId++;
uint256 tokenId = encodeTokenId(collectionId, itemId);
tokenIdToEdition[tokenId] = itemId;
+ BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
_mint(msg.sender, tokenId, 1, "");
emit MemorabiliaRedeemed(msg.sender, tokenId, collectionId, itemId);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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