diff --git a/contracts/drop/DropERC1155.sol b/contracts/drop/DropERC1155.sol index 72860adb1..98e40b180 100644 --- a/contracts/drop/DropERC1155.sol +++ b/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); diff --git a/contracts/drop/DropERC20.sol b/contracts/drop/DropERC20.sol index a496c7e72..7ab2605f3 100644 --- a/contracts/drop/DropERC20.sol +++ b/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(); diff --git a/contracts/drop/DropERC721.sol b/contracts/drop/DropERC721.sol index 2611550b7..44e556218 100644 --- a/contracts/drop/DropERC721.sol +++ b/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. diff --git a/docs/DropERC1155.md b/docs/DropERC1155.md index 16978ff22..ff2b2d5fa 100644 --- a/docs/DropERC1155.md +++ b/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 diff --git a/docs/DropERC721.md b/docs/DropERC721.md index 1393e0734..b3abe591e 100644 --- a/docs/DropERC721.md +++ b/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 diff --git a/lib/forge-std b/lib/forge-std index 9401f0783..bdd321fc4 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 9401f07830e31937c9cfa77890c99c2ee50a5229 +Subproject commit bdd321fc4ac81aff81924effb559427fcccca769 diff --git a/src/test/drop/DropERC721.t.sol b/src/test/drop/DropERC721.t.sol index f2acb352a..36cf79abd 100644 --- a/src/test/drop/DropERC721.t.sol +++ b/src/test/drop/DropERC721.t.sol @@ -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 + ); + } }