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

reentrancy-benign

deadmanxxxii

Root + Impact

Description

  • The Checks-Effects-Interactions (CEI) pattern mandates that all state changes (effects) should be completed before any external calls (interactions) are made, to prevent reentrancy vulnerabilities. In FestivalPass.redeemMemorabilia(uint256),

  • an external call to BeatToken(beatToken).burnFrom occurs before the tokenIdToEdition mapping is updated. Although Slither classifies this as "benign" (meaning it doesn't directly lead to critical asset loss or integrity issues based on current analysis), it still violates the CEI pattern. This can lead to temporary inconsistencies in the contract's state if a re-entering call occurs and attempts to read the tokenIdToEdition mapping before it reflects the final state.

SLITHER OUTPUT:

## reentrancy-benign Impact: Low
Confidence: Medium
- [ ] ID-13 Reentrancy in [FestivalPass.redeemMemorabilia(uint256)](src/FestivalPass.sol#L190-L210): External calls:
- [BeatToken(beatToken).burnFrom(msg.sender,collection.priceInBeat)](src/FestivalPass.sol#L197)
State variables written after the call(s):
- [tokenIdToEdition[tokenId] = itemId](src/FestivalPass.sol#L204)
src/FestivalPass.sol#L190-L210
​
// Root cause in the codebase with @> marks to highlight the relevant section
// src/FestivalPass.sol
function redeemMemorabilia(uint256 editionId) public {
// ... other code ...
BeatToken(beatToken).burnFrom(msg.sender, price); // @> External call (Interaction) occurs here
_mint(msg.sender, editionId, 1, ""); // Another external call
tokenIdToEdition[tokenId] = editionId; // @> State update (Effect) occurs after external calls
// ... other code ...
}
​

Risk

Likelihood:

  • This will occur if a malicious msg.sender provides a BeatToken contract (if they have control over its burnFrom callback, which is uncommon for standard ERC20/BEAT tokens) or an ERC1155 receiver contract that implements re-entrant logic in its callbacks (triggered by _mint).

  • This will occur if a re-entering call reads or depends on the tokenIdToEdition mapping while it still reflects the pre-call state.

Impact:

  • Potential for state inconsistencies if external logic relies on the un-updated tokenIdToEdition mapping during a re-entry.

  • While not immediately exploitable for financial gain or asset loss in this specific context, it represents a deviation from best practices that could become problematic if contract logic changes or if combined with other vulnerabilities.

  • Makes the contract's behavior less predictable under re-entrant conditions.

Proof of Concept

// Example PoC demonstrating benign reentrancy in redeemMemorabilia
// (Conceptual - requires a specific setup where burnFrom or _mint could re-enter)
​
// Scenario:
// 1. User calls `redeemMemorabilia(editionId)`.
// 2. `BeatToken(beatToken).burnFrom` is called.
// 3. If `burnFrom` could trigger a callback (e.g., if BeatToken was also re-entrant)
// and that callback re-enters `redeemMemorabilia` or another function that
// reads `tokenIdToEdition`, the re-entering call would see the old value.
// 4. Similarly, `_mint` can trigger `onERC1155Received` on the recipient.
// If the recipient re-enters, `tokenIdToEdition` is still outdated.
​
// Although the `tokenIdToEdition` is set for the `tokenId` after `_mint` in this specific
// function, the `burnFrom` call occurring before it introduces a window of vulnerability
// if another function could rely on its update. The `_mint` also causes an interaction
// before `tokenIdToEdition` is fully updated.

Recommended Mitigation

- remove this code
+ add this code
​
--- a/src/FestivalPass.sol
+++ b/src/FestivalPass.sol
@@ -196,9 +196,9 @@
require(price > 0, "Collection price must be greater than zero");
require(editionId == itemId, "Invalid edition ID for this collection");
+ tokenIdToEdition[tokenId] = editionId; // @> Move state update before external calls
BeatToken(beatToken).burnFrom(msg.sender, price); // @> External call after state update
_mint(msg.sender, editionId, 1, ""); // @> External call after state update
- tokenIdToEdition[tokenId] = editionId;
emit MemorabiliaRedeemed(msg.sender, editionId, 1);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 25 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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