Merge pull request #155 from thirdweb-dev/drop-bot-check

Drop bot check

Author: nkrishang

contracts/drop/DropERC1155.sol

@@ -59,7 +59,7 @@ contract DropERC1155 is
     uint256 private constant MAX_BPS = 10_000;
 
     /// @dev The thirdweb contract with fee related information.
-    ITWFee public immutable thirdwebFee;
+    ITWFee private immutable thirdwebFee;
 
     /// @dev Owner of the contract (purpose: OpenSea compatibility)
     address private _owner;
@@ -254,6 +254,8 @@ contract DropERC1155 is
         bytes32[] calldata _proofs,
         uint256 _proofMaxQuantityPerTransaction
     ) external payable nonReentrant {
+        require(isTrustedForwarder(msg.sender) || _msgSender() == tx.origin, "BOT");
+
         // Get the active claim condition index.
         uint256 activeConditionId = getActiveClaimConditionId(_tokenId);
 

contracts/drop/DropERC20.sol

@@ -47,7 +47,7 @@ contract DropERC20 is
     bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
 
     /// @dev The thirdweb contract with fee related information.
-    ITWFee internal immutable thirdwebFee;
+    ITWFee private immutable thirdwebFee;
 
     /// @dev Contract level metadata.
     string public contractURI;
@@ -178,6 +178,8 @@ contract DropERC20 is
         bytes32[] calldata _proofs,
         uint256 _proofMaxQuantityPerTransaction
     ) external payable nonReentrant {
+        require(isTrustedForwarder(msg.sender) || _msgSender() == tx.origin, "BOT");
+
         // Get the claim conditions.
         uint256 activeConditionId = getActiveClaimConditionId();
 

contracts/drop/DropERC721.sol

@@ -52,7 +52,7 @@ contract DropERC721 is
     uint256 private constant MAX_BPS = 10_000;
 
     /// @dev The thirdweb contract with fee related information.
-    ITWFee public immutable thirdwebFee;
+    ITWFee private immutable thirdwebFee;
 
     /// @dev Owner of the contract (purpose: OpenSea compatibility)
     address private _owner;
@@ -317,6 +317,8 @@ contract DropERC721 is
         bytes32[] calldata _proofs,
         uint256 _proofMaxQuantityPerTransaction
     ) external payable nonReentrant {
+        require(isTrustedForwarder(msg.sender) || _msgSender() == tx.origin, "BOT");
+
         uint256 tokenIdToClaim = nextTokenIdToClaim;
 
         // Get the claim conditions.

docs/DropERC1155.md

@@ -1008,23 +1008,6 @@ function symbol() external view returns (string)
 |---|---|---|
 | _0 | string | undefined
 
-### thirdwebFee
-
-```solidity
-function thirdwebFee() external view returns (contract ITWFee)
-```
-
-
-
-*The thirdweb contract with fee related information.*
-
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | contract ITWFee | undefined
-
 ### totalSupply
 
 ```solidity

docs/DropERC721.md

@@ -1080,23 +1080,6 @@ function symbol() external view returns (string)
 |---|---|---|
 | _0 | string | undefined
 
-### thirdwebFee
-
-```solidity
-function thirdwebFee() external view returns (contract ITWFee)
-```
-
-
-
-*The thirdweb contract with fee related information.*
-
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | contract ITWFee | undefined
-
 ### tokenByIndex
 
 ```solidity

lib/forge-std

@@ -6,7 +6,54 @@ import "contracts/drop/DropERC721.sol";
 // Test imports
 import "../utils/BaseTest.sol";
 
-contract BaseDropERC721Test is BaseTest {
+contract SubExploitContract is ERC721Holder, ERC1155Holder {
+    DropERC721 internal drop;
+    address payable internal master;
+
+    constructor(address _drop) {
+        drop = DropERC721(_drop);
+        master = payable(msg.sender);
+    }
+
+    /// @dev Lets an account claim NFTs.
+    function claimDrop(
+        address _receiver,
+        uint256 _quantity,
+        address _currency,
+        uint256 _pricePerToken,
+        bytes32[] calldata _proofs,
+        uint256 _proofMaxQuantityPerTransaction
+    ) external {
+        drop.claim(_receiver, _quantity, _currency, _pricePerToken, _proofs, _proofMaxQuantityPerTransaction);
+
+        selfdestruct(master);
+    }
+}
+
+contract MasterExploitContract is ERC721Holder, ERC1155Holder {
+    address internal drop;
+
+    constructor(address _drop) {
+        drop = _drop;
+    }
+
+    /// @dev Lets an account claim NFTs.
+    function performExploit(
+        address _receiver,
+        uint256 _quantity,
+        address _currency,
+        uint256 _pricePerToken,
+        bytes32[] calldata _proofs,
+        uint256 _proofMaxQuantityPerTransaction
+    ) external {
+        for (uint256 i = 0; i < 100; i++) {
+            SubExploitContract sub = new SubExploitContract(address(drop));
+            sub.claimDrop(_receiver, _quantity, _currency, _pricePerToken, _proofs, _proofMaxQuantityPerTransaction);
+        }
+    }
+}
+
+contract DropERC721Test is BaseTest {
     DropERC721 public drop;
 
     function setUp() public override {
@@ -99,7 +146,6 @@ contract BaseDropERC721Test is BaseTest {
     }
 
     function test_claimCondition_waitTimeInSecondsBetweenClaims() public {
-        vm.startPrank(deployer);
         vm.warp(1);
 
         address receiver = getActor(0);
@@ -110,16 +156,20 @@ contract BaseDropERC721Test is BaseTest {
         conditions[0].quantityLimitPerTransaction = 100;
         conditions[0].waitTimeInSecondsBetweenClaims = type(uint256).max;
 
+        vm.prank(deployer);
         drop.lazyMint(100, "ipfs://", bytes(""));
+        vm.prank(deployer);
         drop.setClaimConditions(conditions, false);
+
+        vm.prank(getActor(5), getActor(5));
         drop.claim(receiver, 1, address(0), 0, proofs, 0);
 
         vm.expectRevert("cannot claim.");
+        vm.prank(getActor(5), getActor(5));
         drop.claim(receiver, 1, address(0), 0, proofs, 0);
     }
 
     function test_claimCondition_resetEligibility_waitTimeInSecondsBetweenClaims() public {
-        vm.startPrank(deployer);
         vm.warp(1);
 
         address receiver = getActor(0);
@@ -130,12 +180,47 @@ contract BaseDropERC721Test is BaseTest {
         conditions[0].quantityLimitPerTransaction = 100;
         conditions[0].waitTimeInSecondsBetweenClaims = type(uint256).max;
 
+        vm.prank(deployer);
         drop.lazyMint(100, "ipfs://", bytes(""));
 
+        vm.prank(deployer);
         drop.setClaimConditions(conditions, false);
+
+        vm.prank(getActor(5), getActor(5));
         drop.claim(receiver, 1, address(0), 0, proofs, 0);
 
+        vm.prank(deployer);
         drop.setClaimConditions(conditions, true);
+
+        vm.prank(getActor(5), getActor(5));
         drop.claim(receiver, 1, address(0), 0, proofs, 0);
     }
+
+    function test_multiple_claim_exploit() public {
+        MasterExploitContract masterExploit = new MasterExploitContract(address(drop));
+
+        DropERC721.ClaimCondition[] memory conditions = new DropERC721.ClaimCondition[](1);
+        conditions[0].maxClaimableSupply = 100;
+        conditions[0].quantityLimitPerTransaction = 1;
+        conditions[0].waitTimeInSecondsBetweenClaims = type(uint256).max;
+
+        vm.prank(deployer);
+        drop.lazyMint(100, "ipfs://", bytes(""));
+
+        vm.prank(deployer);
+        drop.setClaimConditions(conditions, false);
+
+        bytes32[] memory proofs = new bytes32[](0);
+
+        vm.startPrank(getActor(5));
+        vm.expectRevert(bytes("BOT"));
+        masterExploit.performExploit(
+            address(masterExploit),
+            conditions[0].quantityLimitPerTransaction,
+            conditions[0].currency,
+            conditions[0].pricePerToken,
+            proofs,
+            0
+        );
+    }
 }