From 3b6dacf81d30d425732ebb19fcaac0d76d4eb936 Mon Sep 17 00:00:00 2001 From: Krishang Date: Thu, 25 Jan 2024 17:52:28 +0530 Subject: [PATCH] Add nonReentrant to possible reentrant fn calls --- contracts/base/ERC1155SignatureMint.sol | 6 +++--- contracts/base/ERC20SignatureMint.sol | 6 +++--- contracts/base/ERC20SignatureMintVote.sol | 6 +++--- contracts/base/ERC721Multiwrap.sol | 16 +++++++++++++--- contracts/base/ERC721SignatureMint.sol | 6 +++--- contracts/prebuilts/loyalty/LoyaltyCard.sol | 5 ++++- .../english-auctions/EnglishAuctionsLogic.sol | 6 ++++-- contracts/prebuilts/pack/Pack.sol | 2 +- contracts/prebuilts/split/Split.sol | 12 +++++++----- contracts/prebuilts/staking/EditionStake.sol | 2 +- contracts/prebuilts/staking/NFTStake.sol | 2 +- contracts/prebuilts/token/TokenERC1155.sol | 2 +- contracts/prebuilts/token/TokenERC20.sol | 2 +- contracts/prebuilts/token/TokenERC721.sol | 2 +- .../extension/BurnToClaimDrop721Logic.sol | 6 ++++-- 15 files changed, 50 insertions(+), 31 deletions(-) diff --git a/contracts/base/ERC1155SignatureMint.sol b/contracts/base/ERC1155SignatureMint.sol index 3ccee476f..b7a74af24 100644 --- a/contracts/base/ERC1155SignatureMint.sol +++ b/contracts/base/ERC1155SignatureMint.sol @@ -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"; /** @@ -23,7 +23,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol"; * */ -contract ERC1155SignatureMint is ERC1155Base, PrimarySale, SignatureMintERC1155 { +contract ERC1155SignatureMint is ERC1155Base, PrimarySale, SignatureMintERC1155, ReentrancyGuard { /*////////////////////////////////////////////////////////////// Constructor //////////////////////////////////////////////////////////////*/ @@ -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; diff --git a/contracts/base/ERC20SignatureMint.sol b/contracts/base/ERC20SignatureMint.sol index a3a2a2f00..6090aabd1 100644 --- a/contracts/base/ERC20SignatureMint.sol +++ b/contracts/base/ERC20SignatureMint.sol @@ -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"; /** @@ -23,7 +23,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol"; * */ -contract ERC20SignatureMint is ERC20Base, PrimarySale, SignatureMintERC20 { +contract ERC20SignatureMint is ERC20Base, PrimarySale, SignatureMintERC20, ReentrancyGuard { /*////////////////////////////////////////////////////////////// Constructor //////////////////////////////////////////////////////////////*/ @@ -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. diff --git a/contracts/base/ERC20SignatureMintVote.sol b/contracts/base/ERC20SignatureMintVote.sol index 80dae73a3..f0d03542e 100644 --- a/contracts/base/ERC20SignatureMintVote.sol +++ b/contracts/base/ERC20SignatureMintVote.sol @@ -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"; /** @@ -23,7 +23,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol"; * */ -contract ERC20SignatureMintVote is ERC20Vote, PrimarySale, SignatureMintERC20 { +contract ERC20SignatureMintVote is ERC20Vote, PrimarySale, SignatureMintERC20, ReentrancyGuard { /*////////////////////////////////////////////////////////////// Constructor //////////////////////////////////////////////////////////////*/ @@ -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. diff --git a/contracts/base/ERC721Multiwrap.sol b/contracts/base/ERC721Multiwrap.sol index a2ee916f1..6125b059f 100644 --- a/contracts/base/ERC721Multiwrap.sol +++ b/contracts/base/ERC721Multiwrap.sol @@ -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 @@ -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 //////////////////////////////////////////////////////////////*/ @@ -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); @@ -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."); diff --git a/contracts/base/ERC721SignatureMint.sol b/contracts/base/ERC721SignatureMint.sol index e6ec12e4f..a3b389b00 100644 --- a/contracts/base/ERC721SignatureMint.sol +++ b/contracts/base/ERC721SignatureMint.sol @@ -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"; /** @@ -24,7 +24,7 @@ import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol"; * */ -contract ERC721SignatureMint is ERC721Base, PrimarySale, SignatureMintERC721 { +contract ERC721SignatureMint is ERC721Base, PrimarySale, SignatureMintERC721, ReentrancyGuard { /*////////////////////////////////////////////////////////////// Constructor //////////////////////////////////////////////////////////////*/ @@ -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(); diff --git a/contracts/prebuilts/loyalty/LoyaltyCard.sol b/contracts/prebuilts/loyalty/LoyaltyCard.sol index 7f986a352..a841aab17 100644 --- a/contracts/prebuilts/loyalty/LoyaltyCard.sol +++ b/contracts/prebuilts/loyalty/LoyaltyCard.sol @@ -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); } diff --git a/contracts/prebuilts/marketplace/english-auctions/EnglishAuctionsLogic.sol b/contracts/prebuilts/marketplace/english-auctions/EnglishAuctionsLogic.sol index 66574a050..89a56f756 100644 --- a/contracts/prebuilts/marketplace/english-auctions/EnglishAuctionsLogic.sol +++ b/contracts/prebuilts/marketplace/english-auctions/EnglishAuctionsLogic.sol @@ -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); @@ -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]; diff --git a/contracts/prebuilts/pack/Pack.sol b/contracts/prebuilts/pack/Pack.sol index 0b427bc75..cdd9f2ef7 100644 --- a/contracts/prebuilts/pack/Pack.sol +++ b/contracts/prebuilts/pack/Pack.sol @@ -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"); diff --git a/contracts/prebuilts/split/Split.sol b/contracts/prebuilts/split/Split.sol index 69bc32bcd..028f5a0ee 100644 --- a/contracts/prebuilts/split/Split.sol +++ b/contracts/prebuilts/split/Split.sol @@ -25,6 +25,7 @@ import "@openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgrad // Utils import "../../extension/Multicall.sol"; import "../../lib/FeeType.sol"; +import "../../extension/upgradeable/ReentrancyGuard.sol"; contract Split is IThirdwebContract, @@ -32,7 +33,8 @@ contract Split is Multicall, ERC2771ContextUpgradeable, AccessControlEnumerableUpgradeable, - PaymentSplitterUpgradeable + PaymentSplitterUpgradeable, + ReentrancyGuard { bytes32 private constant MODULE_TYPE = bytes32("Split"); uint128 private constant VERSION = 1; @@ -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"); } @@ -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"); } @@ -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))); @@ -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)); diff --git a/contracts/prebuilts/staking/EditionStake.sol b/contracts/prebuilts/staking/EditionStake.sol index 67f8aea42..9f5375bbc 100644 --- a/contracts/prebuilts/staking/EditionStake.sol +++ b/contracts/prebuilts/staking/EditionStake.sol @@ -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 diff --git a/contracts/prebuilts/staking/NFTStake.sol b/contracts/prebuilts/staking/NFTStake.sol index 32d5a6f9d..c50794ffc 100644 --- a/contracts/prebuilts/staking/NFTStake.sol +++ b/contracts/prebuilts/staking/NFTStake.sol @@ -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 diff --git a/contracts/prebuilts/token/TokenERC1155.sol b/contracts/prebuilts/token/TokenERC1155.sol index fd2375eed..cbee51b4b 100644 --- a/contracts/prebuilts/token/TokenERC1155.sol +++ b/contracts/prebuilts/token/TokenERC1155.sol @@ -211,7 +211,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; diff --git a/contracts/prebuilts/token/TokenERC20.sol b/contracts/prebuilts/token/TokenERC20.sol index 4ff5898b1..c41498174 100644 --- a/contracts/prebuilts/token/TokenERC20.sol +++ b/contracts/prebuilts/token/TokenERC20.sol @@ -158,7 +158,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); } diff --git a/contracts/prebuilts/token/TokenERC721.sol b/contracts/prebuilts/token/TokenERC721.sol index 13f977c6b..163a713e2 100644 --- a/contracts/prebuilts/token/TokenERC721.sol +++ b/contracts/prebuilts/token/TokenERC721.sol @@ -187,7 +187,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); } diff --git a/contracts/prebuilts/unaudited/burn-to-claim-drop/extension/BurnToClaimDrop721Logic.sol b/contracts/prebuilts/unaudited/burn-to-claim-drop/extension/BurnToClaimDrop721Logic.sol index 77a14a5b9..80d1411f6 100644 --- a/contracts/prebuilts/unaudited/burn-to-claim-drop/extension/BurnToClaimDrop721Logic.sol +++ b/contracts/prebuilts/unaudited/burn-to-claim-drop/extension/BurnToClaimDrop721Logic.sol @@ -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, @@ -45,7 +46,8 @@ contract BurnToClaimDrop721Logic is LazyMint, Drop, ERC2771ContextUpgradeable, - ERC721AUpgradeable + ERC721AUpgradeable, + ReentrancyGuard { using Strings for uint256; @@ -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