Skip to content

Commit

Permalink
Resolve: Add nonReentrant modifier to relevant base contract functions
Browse files Browse the repository at this point in the history
…#601 (#611)

Add nonReentrant to possible reentrant fn calls
  • Loading branch information
nkrishang authored Jan 25, 2024
1 parent ed8b329 commit da53846
Show file tree
Hide file tree
Showing 15 changed files with 50 additions and 31 deletions.
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

0 comments on commit da53846

Please sign in to comment.