Skip to content

Resolve: Add nonReentrant modifier to relevant base contract functions #601 #611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions contracts/base/ERC1155SignatureMint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "./ERC1155Base.sol";

import "../extension/PrimarySale.sol";
import "../extension/SignatureMintERC1155.sol";

import { ReentrancyGuard } from "../extension/upgradeable/ReentrancyGuard.sol";
import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";

/**
Expand All @@ -23,7 +23,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";
*
*/

contract ERC1155SignatureMint is ERC1155Base, PrimarySale, SignatureMintERC1155 {
contract ERC1155SignatureMint is ERC1155Base, PrimarySale, SignatureMintERC1155, ReentrancyGuard {
/*//////////////////////////////////////////////////////////////
Constructor
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -52,7 +52,7 @@ contract ERC1155SignatureMint is ERC1155Base, PrimarySale, SignatureMintERC1155
function mintWithSignature(
MintRequest calldata _req,
bytes calldata _signature
) external payable virtual override returns (address signer) {
) external payable virtual override nonReentrant returns (address signer) {
require(_req.quantity > 0, "Minting zero tokens.");

uint256 tokenIdToMint;
Expand Down
6 changes: 3 additions & 3 deletions contracts/base/ERC20SignatureMint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "./ERC20Base.sol";

import "../extension/PrimarySale.sol";
import { SignatureMintERC20 } from "../extension/SignatureMintERC20.sol";

import { ReentrancyGuard } from "../extension/upgradeable/ReentrancyGuard.sol";
import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";

/**
Expand All @@ -23,7 +23,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";
*
*/

contract ERC20SignatureMint is ERC20Base, PrimarySale, SignatureMintERC20 {
contract ERC20SignatureMint is ERC20Base, PrimarySale, SignatureMintERC20, ReentrancyGuard {
/*//////////////////////////////////////////////////////////////
Constructor
//////////////////////////////////////////////////////////////*/
Expand All @@ -50,7 +50,7 @@ contract ERC20SignatureMint is ERC20Base, PrimarySale, SignatureMintERC20 {
function mintWithSignature(
MintRequest calldata _req,
bytes calldata _signature
) external payable virtual returns (address signer) {
) external payable virtual nonReentrant returns (address signer) {
require(_req.quantity > 0, "Minting zero tokens.");

// Verify and process payload.
Expand Down
6 changes: 3 additions & 3 deletions contracts/base/ERC20SignatureMintVote.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "./ERC20Vote.sol";

import "../extension/PrimarySale.sol";
import { SignatureMintERC20 } from "../extension/SignatureMintERC20.sol";

import { ReentrancyGuard } from "../extension/upgradeable/ReentrancyGuard.sol";
import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";

/**
Expand All @@ -23,7 +23,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";
*
*/

contract ERC20SignatureMintVote is ERC20Vote, PrimarySale, SignatureMintERC20 {
contract ERC20SignatureMintVote is ERC20Vote, PrimarySale, SignatureMintERC20, ReentrancyGuard {
/*//////////////////////////////////////////////////////////////
Constructor
//////////////////////////////////////////////////////////////*/
Expand All @@ -50,7 +50,7 @@ contract ERC20SignatureMintVote is ERC20Vote, PrimarySale, SignatureMintERC20 {
function mintWithSignature(
MintRequest calldata _req,
bytes calldata _signature
) external payable virtual returns (address signer) {
) external payable virtual nonReentrant returns (address signer) {
require(_req.quantity > 0, "Minting zero tokens.");

// Verify and process payload.
Expand Down
16 changes: 13 additions & 3 deletions contracts/base/ERC721Multiwrap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import "../extension/Royalty.sol";
import "../extension/SoulboundERC721A.sol";
import "../extension/TokenStore.sol";
import "../extension/Multicall.sol";
import { ReentrancyGuard } from "../extension/upgradeable/ReentrancyGuard.sol";

/**
* BASE: ERC721Base
Expand All @@ -26,7 +27,16 @@ import "../extension/Multicall.sol";
*
*/

contract ERC721Multiwrap is Multicall, TokenStore, SoulboundERC721A, ERC721A, ContractMetadata, Ownable, Royalty {
contract ERC721Multiwrap is
Multicall,
TokenStore,
SoulboundERC721A,
ERC721A,
ContractMetadata,
Ownable,
Royalty,
ReentrancyGuard
{
/*//////////////////////////////////////////////////////////////
Permission control roles
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -148,7 +158,7 @@ contract ERC721Multiwrap is Multicall, TokenStore, SoulboundERC721A, ERC721A, Co
Token[] calldata _tokensToWrap,
string calldata _uriForWrappedToken,
address _recipient
) public payable virtual onlyRoleWithSwitch(MINTER_ROLE) returns (uint256 tokenId) {
) public payable virtual onlyRoleWithSwitch(MINTER_ROLE) nonReentrant returns (uint256 tokenId) {
if (!hasRole(ASSET_ROLE, address(0))) {
for (uint256 i = 0; i < _tokensToWrap.length; i += 1) {
_checkRole(ASSET_ROLE, _tokensToWrap[i].assetContract);
Expand All @@ -170,7 +180,7 @@ contract ERC721Multiwrap is Multicall, TokenStore, SoulboundERC721A, ERC721A, Co
* @param _tokenId The token Id of the wrapped NFT to unwrap.
* @param _recipient The recipient of the underlying ERC1155, ERC721, ERC20 tokens of the wrapped NFT.
*/
function unwrap(uint256 _tokenId, address _recipient) public virtual onlyRoleWithSwitch(UNWRAP_ROLE) {
function unwrap(uint256 _tokenId, address _recipient) public virtual onlyRoleWithSwitch(UNWRAP_ROLE) nonReentrant {
require(_tokenId < nextTokenIdToMint(), "wrapped NFT DNE.");
require(isApprovedOrOwner(msg.sender, _tokenId), "caller not approved for unwrapping.");

Expand Down
6 changes: 3 additions & 3 deletions contracts/base/ERC721SignatureMint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import "./ERC721Base.sol";
import "../extension/PrimarySale.sol";
import "../extension/PermissionsEnumerable.sol";
import "../extension/SignatureMintERC721.sol";

import { ReentrancyGuard } from "../extension/upgradeable/ReentrancyGuard.sol";
import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";

/**
Expand All @@ -24,7 +24,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol";
*
*/

contract ERC721SignatureMint is ERC721Base, PrimarySale, SignatureMintERC721 {
contract ERC721SignatureMint is ERC721Base, PrimarySale, SignatureMintERC721, ReentrancyGuard {
/*//////////////////////////////////////////////////////////////
Constructor
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -53,7 +53,7 @@ contract ERC721SignatureMint is ERC721Base, PrimarySale, SignatureMintERC721 {
function mintWithSignature(
MintRequest calldata _req,
bytes calldata _signature
) external payable virtual override returns (address signer) {
) external payable virtual override nonReentrant returns (address signer) {
require(_req.quantity == 1, "quantiy must be 1");

uint256 tokenIdToMint = nextTokenIdToMint();
Expand Down
5 changes: 4 additions & 1 deletion contracts/prebuilts/loyalty/LoyaltyCard.sol
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ contract LoyaltyCard is
}

/// @dev Lets an account with MINTER_ROLE mint an NFT. Always mints 1 NFT.
function mintTo(address _to, string calldata _uri) external onlyRole(MINTER_ROLE) returns (uint256 tokenIdMinted) {
function mintTo(
address _to,
string calldata _uri
) external onlyRole(MINTER_ROLE) nonReentrant returns (uint256 tokenIdMinted) {
tokenIdMinted = _mintTo(_to, _uri);
emit TokensMinted(_to, tokenIdMinted, _uri);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ contract EnglishAuctionsLogic is IEnglishAuctions, ReentrancyGuard, ERC2771Conte
/// @notice Auction ERC721 or ERC1155 NFTs.
function createAuction(
AuctionParameters calldata _params
) external onlyListerRole onlyAssetRole(_params.assetContract) returns (uint256 auctionId) {
) external onlyListerRole onlyAssetRole(_params.assetContract) nonReentrant returns (uint256 auctionId) {
auctionId = _getNextAuctionId();
address auctionCreator = _msgSender();
TokenType tokenType = _getTokenType(_params.assetContract);
Expand Down Expand Up @@ -181,7 +181,9 @@ contract EnglishAuctionsLogic is IEnglishAuctions, ReentrancyGuard, ERC2771Conte
}

/// @dev Cancels an auction.
function cancelAuction(uint256 _auctionId) external onlyExistingAuction(_auctionId) onlyAuctionCreator(_auctionId) {
function cancelAuction(
uint256 _auctionId
) external onlyExistingAuction(_auctionId) onlyAuctionCreator(_auctionId) nonReentrant {
Auction memory _targetAuction = _englishAuctionsStorage().auctions[_auctionId];
Bid memory _winningBid = _englishAuctionsStorage().winningBid[_auctionId];

Expand Down
2 changes: 1 addition & 1 deletion contracts/prebuilts/pack/Pack.sol
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ contract Pack is
}

/// @notice Lets a pack owner open packs and receive the packs' reward units.
function openPack(uint256 _packId, uint256 _amountToOpen) external returns (Token[] memory) {
function openPack(uint256 _packId, uint256 _amountToOpen) external nonReentrant returns (Token[] memory) {
address opener = _msgSender();

require(isTrustedForwarder(msg.sender) || opener == tx.origin, "!EOA");
Expand Down
12 changes: 7 additions & 5 deletions contracts/prebuilts/split/Split.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ import "@openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgrad
// Utils
import "../../extension/Multicall.sol";
import "../../lib/FeeType.sol";
import "../../extension/upgradeable/ReentrancyGuard.sol";

contract Split is
IThirdwebContract,
Initializable,
Multicall,
ERC2771ContextUpgradeable,
AccessControlEnumerableUpgradeable,
PaymentSplitterUpgradeable
PaymentSplitterUpgradeable,
ReentrancyGuard
{
bytes32 private constant MODULE_TYPE = bytes32("Split");
uint128 private constant VERSION = 1;
Expand Down Expand Up @@ -76,7 +78,7 @@ contract Split is
* @dev Triggers a transfer to `account` of the amount of Ether they are owed, according to their percentage of the
* total shares and their previous withdrawals.
*/
function release(address payable account) public virtual override {
function release(address payable account) public virtual override nonReentrant {
uint256 payment = _release(account);
require(payment != 0, "PaymentSplitter: account is not due payment");
}
Expand All @@ -86,7 +88,7 @@ contract Split is
* percentage of the total shares and their previous withdrawals. `token` must be the address of an IERC20
* contract.
*/
function release(IERC20Upgradeable token, address account) public virtual override {
function release(IERC20Upgradeable token, address account) public virtual override nonReentrant {
uint256 payment = _release(token, account);
require(payment != 0, "PaymentSplitter: account is not due payment");
}
Expand Down Expand Up @@ -134,7 +136,7 @@ contract Split is
/**
* @dev Release the owed amount of token to all of the payees.
*/
function distribute() public virtual {
function distribute() public virtual nonReentrant {
uint256 count = payeeCount();
for (uint256 i = 0; i < count; i++) {
_release(payable(payee(i)));
Expand All @@ -144,7 +146,7 @@ contract Split is
/**
* @dev Release owed amount of the `token` to all of the payees.
*/
function distribute(IERC20Upgradeable token) public virtual {
function distribute(IERC20Upgradeable token) public virtual nonReentrant {
uint256 count = payeeCount();
for (uint256 i = 0; i < count; i++) {
_release(token, payee(i));
Expand Down
2 changes: 1 addition & 1 deletion contracts/prebuilts/staking/EditionStake.sol
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ contract EditionStake is
}

/// @dev Admin can withdraw excess reward tokens.
function withdrawRewardTokens(uint256 _amount) external {
function withdrawRewardTokens(uint256 _amount) external nonReentrant {
require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()), "Not authorized");

// to prevent locking of direct-transferred tokens
Expand Down
2 changes: 1 addition & 1 deletion contracts/prebuilts/staking/NFTStake.sol
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ contract NFTStake is
}

/// @dev Admin can withdraw excess reward tokens.
function withdrawRewardTokens(uint256 _amount) external {
function withdrawRewardTokens(uint256 _amount) external nonReentrant {
require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()), "Not authorized");

// to prevent locking of direct-transferred tokens
Expand Down
2 changes: 1 addition & 1 deletion contracts/prebuilts/token/TokenERC1155.sol
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ contract TokenERC1155 is
uint256 _tokenId,
string calldata _uri,
uint256 _amount
) external onlyRole(MINTER_ROLE) {
) external nonReentrant onlyRole(MINTER_ROLE) {
uint256 tokenIdToMint;
if (_tokenId == type(uint256).max) {
tokenIdToMint = nextTokenIdToMint;
Expand Down
2 changes: 1 addition & 1 deletion contracts/prebuilts/token/TokenERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ contract TokenERC20 is
*
* - the caller must have the `MINTER_ROLE`.
*/
function mintTo(address to, uint256 amount) public virtual {
function mintTo(address to, uint256 amount) public virtual nonReentrant {
require(hasRole(MINTER_ROLE, _msgSender()), "not minter.");
_mintTo(to, amount);
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/prebuilts/token/TokenERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ contract TokenERC721 is
}

/// @dev Lets an account with MINTER_ROLE mint an NFT.
function mintTo(address _to, string calldata _uri) external onlyRole(MINTER_ROLE) returns (uint256) {
function mintTo(address _to, string calldata _uri) external nonReentrant onlyRole(MINTER_ROLE) returns (uint256) {
// `_mintTo` is re-used. `mintTo` just adds a minter role check.
return _mintTo(_to, _uri);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { ContractMetadata } from "../../../../extension/upgradeable/ContractMeta
import { Ownable } from "../../../../extension/upgradeable/Ownable.sol";
import { PermissionsStorage } from "../../../../extension/upgradeable/Permissions.sol";
import { BurnToClaim, BurnToClaimStorage } from "../../../../extension/upgradeable/BurnToClaim.sol";
import { ReentrancyGuard } from "../../../../extension/upgradeable/ReentrancyGuard.sol";

contract BurnToClaimDrop721Logic is
ContractMetadata,
Expand All @@ -45,7 +46,8 @@ contract BurnToClaimDrop721Logic is
LazyMint,
Drop,
ERC2771ContextUpgradeable,
ERC721AUpgradeable
ERC721AUpgradeable,
ReentrancyGuard
{
using Strings for uint256;

Expand Down Expand Up @@ -137,7 +139,7 @@ contract BurnToClaimDrop721Logic is
//////////////////////////////////////////////////////////////*/

/// @notice Claim lazy minted tokens after burning required tokens from origin contract.
function burnAndClaim(uint256 _burnTokenId, uint256 _quantity) external payable {
function burnAndClaim(uint256 _burnTokenId, uint256 _quantity) external payable nonReentrant {
_checkTokenSupply(_quantity);

// Verify and burn tokens on origin contract
Expand Down