Skip to content

Commit 95e1be8

Browse files
nkrishangnkrishang
authored
TieredDrop audit fixes (#281)
* Add validation test for H1 * Fix H1: Out of order lazy minting in TieredDrop causes incorrect claims * forge updates * Fix H1:2 * Fix H1_2 validation test comments * Fix (G-2) Loop variables
1 parent f210af1 commit 95e1be8

File tree

3 files changed

+182
-14
lines changed

3 files changed

+182
-14
lines changed

‎contracts/tiered-drop/TieredDrop.sol‎

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ contract TieredDrop is
206206
totalRemainingInTier[_tier] += _amount;
207207

208208
uint256 startId = nextTokenIdToLazyMint;
209-
if (isTierEmpty(_tier)) {
209+
if (isTierEmpty(_tier) || nextMetadataIdToMapFromTier[_tier] == type(uint256).max) {
210210
nextMetadataIdToMapFromTier[_tier] = startId;
211211
}
212212

@@ -275,8 +275,6 @@ contract TieredDrop is
275275
transferTokensOnClaim(to, quantity, tiersInPriority);
276276

277277
emit TokensClaimed(_msgSender(), to, tokenIdToMint, quantity, tiersInPriority);
278-
279-
// emit RequestExecuted(_msgSender(), signer, _req);
280278
}
281279

282280
/*///////////////////////////////////////////////////////////////
@@ -348,18 +346,51 @@ contract TieredDrop is
348346
uint256 _startIdToMap,
349347
uint256 _quantity
350348
) private {
351-
uint256 proxyStartId = nextMetadataIdToMapFromTier[_tier];
352-
uint256 proxyEndId = proxyStartId + _quantity;
349+
uint256 nextIdFromTier = nextMetadataIdToMapFromTier[_tier];
350+
uint256 startTokenId = _startIdToMap;
351+
352+
TokenRange[] memory tokensInTier = tokensInTier[_tier];
353+
uint256 len = tokensInTier.length;
354+
355+
uint256 qtyRemaining = _quantity;
356+
357+
for (uint256 i = 0; i < len; i += 1) {
358+
TokenRange memory range = tokensInTier[i];
359+
uint256 gap = 0;
360+
361+
if (range.startIdInclusive <= nextIdFromTier && nextIdFromTier < range.endIdNonInclusive) {
362+
uint256 proxyStartId = nextIdFromTier;
363+
uint256 proxyEndId = proxyStartId + qtyRemaining <= range.endIdNonInclusive
364+
? proxyStartId + qtyRemaining
365+
: range.endIdNonInclusive;
366+
367+
gap = proxyEndId - proxyStartId;
353368

354-
uint256 endTokenId = _startIdToMap + _quantity;
369+
uint256 endTokenId = startTokenId + gap;
355370

356-
endIdsAtMint[lengthEndIdsAtMint] = endTokenId;
357-
lengthEndIdsAtMint += 1;
371+
endIdsAtMint[lengthEndIdsAtMint] = endTokenId;
372+
lengthEndIdsAtMint += 1;
358373

359-
tierAtEndId[endTokenId] = _tier;
360-
proxyTokenRange[endTokenId] = TokenRange(proxyStartId, proxyEndId);
374+
tierAtEndId[endTokenId] = _tier;
375+
proxyTokenRange[endTokenId] = TokenRange(proxyStartId, proxyEndId);
361376

362-
nextMetadataIdToMapFromTier[_tier] += _quantity;
377+
startTokenId += gap;
378+
qtyRemaining -= gap;
379+
380+
if (nextIdFromTier + gap < range.endIdNonInclusive) {
381+
nextIdFromTier += gap;
382+
} else if (i < (len - 1)) {
383+
nextIdFromTier = tokensInTier[i + 1].startIdInclusive;
384+
} else {
385+
nextIdFromTier = type(uint256).max;
386+
}
387+
}
388+
389+
if (qtyRemaining == 0) {
390+
nextMetadataIdToMapFromTier[_tier] = nextIdFromTier;
391+
break;
392+
}
393+
}
363394
}
364395

365396
/// @dev Returns how much of the total-quantity-to-distribute can come from the given tier.
@@ -408,10 +439,10 @@ contract TieredDrop is
408439
require(_startIdx < _endIdx && _endIdx <= len, "TieredDrop: invalid indices.");
409440

410441
uint256 numOfRangesForTier = 0;
442+
bytes32 hashOfTier = keccak256(abi.encodePacked(_tier));
411443

412444
for (uint256 i = _startIdx; i < _endIdx; i += 1) {
413445
bytes32 hashOfStoredTier = keccak256(abi.encodePacked(tierAtEndId[endIdsAtMint[i]]));
414-
bytes32 hashOfTier = keccak256(abi.encodePacked(_tier));
415446

416447
if (hashOfStoredTier == hashOfTier) {
417448
numOfRangesForTier += 1;
@@ -423,7 +454,6 @@ contract TieredDrop is
423454

424455
for (uint256 i = _startIdx; i < _endIdx; i += 1) {
425456
bytes32 hashOfStoredTier = keccak256(abi.encodePacked(tierAtEndId[endIdsAtMint[i]]));
426-
bytes32 hashOfTier = keccak256(abi.encodePacked(_tier));
427457

428458
if (hashOfStoredTier == hashOfTier) {
429459
uint256 end = endIdsAtMint[i];

‎lib/forge-std‎

‎src/test/TieredDrop.t.sol‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,144 @@ contract TieredDropTest is BaseTest {
799799
// assertEq(baseURIs3.length, 1);
800800
// assertEq(baseURIs3[0], baseURITier3);
801801
// }
802+
803+
////////////////////////////////////////////////
804+
// //
805+
// audit tests //
806+
// //
807+
////////////////////////////////////////////////
808+
809+
function test_state_claimWithSignature_IssueH1() public {
810+
// Lazy mint tokens: 3 different tiers
811+
vm.startPrank(dropAdmin);
812+
813+
// Tier 1: tokenIds assigned 0 -> 10 non-inclusive.
814+
tieredDrop.lazyMint(quantityTier1, baseURITier1, tier1, "");
815+
// Tier 2: tokenIds assigned 10 -> 20 non-inclusive.
816+
tieredDrop.lazyMint(10, baseURITier2, tier2, "");
817+
// Tier 3: tokenIds assigned 20 -> 50 non-inclusive.
818+
tieredDrop.lazyMint(quantityTier3, baseURITier3, tier3, "");
819+
820+
// Tier 2: tokenIds assigned 50 -> 60 non-inclusive.
821+
tieredDrop.lazyMint(quantityTier2 - 10, baseURITier2, tier2, "");
822+
823+
vm.stopPrank();
824+
825+
string[] memory tiers = new string[](2);
826+
tiers[0] = tier2;
827+
tiers[1] = tier1;
828+
829+
uint256 claimQuantity = 25;
830+
831+
_setupClaimSignature(tiers, claimQuantity);
832+
833+
assertEq(tieredDrop.hasRole(keccak256("MINTER_ROLE"), deployerSigner), true);
834+
835+
vm.warp(claimRequest.validityStartTimestamp);
836+
vm.prank(claimer);
837+
tieredDrop.claimWithSignature(claimRequest, claimSignature);
838+
assertEq(tieredDrop.balanceOf(claimer), claimQuantity);
839+
840+
for (uint256 i = 0; i < claimQuantity; i += 1) {
841+
// Outputs:
842+
// Checking 0 baseURI2/10
843+
// Checking 1 baseURI2/11
844+
// Checking 2 baseURI2/12
845+
// Checking 3 baseURI2/13
846+
// Checking 4 baseURI2/14
847+
// Checking 5 baseURI2/15
848+
// Checking 6 baseURI2/16
849+
// Checking 7 baseURI2/17
850+
// Checking 8 baseURI2/18
851+
// Checking 9 baseURI2/19
852+
// Checking 10 baseURI3/50
853+
// Checking 11 baseURI3/51
854+
// Checking 12 baseURI3/52
855+
// Checking 13 baseURI3/53
856+
// Checking 14 baseURI3/54
857+
// Checking 15 baseURI3/55
858+
// Checking 16 baseURI3/56
859+
// Checking 17 baseURI3/57
860+
// Checking 18 baseURI3/58
861+
// Checking 19 baseURI3/59
862+
// Checking 20 baseURI1/0
863+
// Checking 21 baseURI1/1
864+
// Checking 22 baseURI1/2
865+
// Checking 23 baseURI1/3
866+
// Checking 24 baseURI1/4
867+
console.log("Checking", i, tieredDrop.tokenURI(i));
868+
}
869+
}
870+
871+
function test_state_claimWithSignature_IssueH1_2() public {
872+
// Lazy mint tokens: 3 different tiers
873+
vm.startPrank(dropAdmin);
874+
875+
// Tier 1: tokenIds assigned 0 -> 10 non-inclusive.
876+
tieredDrop.lazyMint(quantityTier1, baseURITier1, tier1, "");
877+
// Tier 2: tokenIds assigned 10 -> 20 non-inclusive.
878+
tieredDrop.lazyMint(1, baseURITier2, tier2, ""); // 10 -> 11
879+
tieredDrop.lazyMint(9, baseURITier2, tier2, ""); // 11 -> 20
880+
// Tier 3: tokenIds assigned 20 -> 50 non-inclusive.
881+
tieredDrop.lazyMint(quantityTier3, baseURITier3, tier3, "");
882+
883+
// Tier 2: tokenIds assigned 50 -> 60 non-inclusive.
884+
tieredDrop.lazyMint(quantityTier2 - 10, baseURITier2, tier2, "");
885+
886+
vm.stopPrank();
887+
888+
string[] memory tiers = new string[](2);
889+
tiers[0] = tier2;
890+
tiers[1] = tier1;
891+
892+
uint256[3] memory claimQuantities = [uint256(1), uint256(3), uint256(21)];
893+
uint256 claimedCount = 0;
894+
for (uint256 loop = 0; loop < 3; loop++) {
895+
uint256 claimQuantity = claimQuantities[loop];
896+
uint256 offset = claimedCount;
897+
898+
_setupClaimSignature(tiers, claimQuantity);
899+
900+
assertEq(tieredDrop.hasRole(keccak256("MINTER_ROLE"), deployerSigner), true);
901+
902+
vm.warp(claimRequest.validityStartTimestamp);
903+
vm.prank(claimer);
904+
tieredDrop.claimWithSignature(claimRequest, claimSignature);
905+
906+
claimedCount += claimQuantity;
907+
assertEq(tieredDrop.balanceOf(claimer), claimedCount);
908+
909+
for (uint256 i = offset; i < claimQuantity + (offset); i += 1) {
910+
// Outputs:
911+
// Checking 0 baseURI2/10
912+
// Checking 1 baseURI2/11
913+
// Checking 2 baseURI2/12
914+
// Checking 3 baseURI2/13
915+
// Checking 4 baseURI2/14
916+
// Checking 5 baseURI2/15
917+
// Checking 6 baseURI2/16
918+
// Checking 7 baseURI2/17
919+
// Checking 8 baseURI2/18
920+
// Checking 9 baseURI2/19
921+
// Checking 10 baseURI3/50
922+
// Checking 11 baseURI3/51
923+
// Checking 12 baseURI3/52
924+
// Checking 13 baseURI3/53
925+
// Checking 14 baseURI3/54
926+
// Checking 15 baseURI3/55
927+
// Checking 16 baseURI3/56
928+
// Checking 17 baseURI3/57
929+
// Checking 18 baseURI3/58
930+
// Checking 19 baseURI3/59
931+
// Checking 20 baseURI1/0
932+
// Checking 21 baseURI1/1
933+
// Checking 22 baseURI1/2
934+
// Checking 23 baseURI1/3
935+
// Checking 24 baseURI1/4
936+
console.log("Checking", i, tieredDrop.tokenURI(i));
937+
}
938+
}
939+
}
802940
}
803941

804942
contract TieredDropBechmarkTest is BaseTest {

0 commit comments

Comments
 (0)