SilentDefendersOfDeFi - reserved Token minters can be prevented from Minting all their tokens

[sherlock-admin]

SilentDefendersOfDeFi

medium

reserved Token minters can be prevented from Minting all their tokens

Summary

When a DAO is migrated to L2, the former holders can be allowed to mint their tokens from a specified reserve during an initial mint period. However, a malicious user or attacker can prevent them from doing so, in time.

Vulnerability Detail

When a DAO is migrated to L2, there is a reserve and the MerkleMinterconfigured, to enable former token holders to mint new Tokens from the reserve in a given period of time, using a MerkleProof.

The intended way for them to do this is to call mintFromReserve(address tokenContract, MerkleClaim[] calldata claims) on the MerkleReserveMinter contract. Here they have to pass, the corresponding tokenContract, and an array of MerkleClaims, which consists of:

• an address to mint to
• the tokenId to mint
• The Merkle proof

A user that has many tokens to mint, probably wants to mint them all in one call. However, an malicious user or attacker, can wait for the transaction to be in the mempool, see the actual merkleProofs, and then frontrun it with a transaction minting only 1 of the tokens. This would result in the initially created transaction to mint all the tokens, to be reverted, as one of the tokens could not be minted.

Example: Alice is able to claim a total of 50 tokens (id:0-49) and has the proves for them. Bob only has fewer tokens, and wants Alice to receive less tokens on the new DAO. Therefore he monitors the mempool.

1. Alice creates a transaction to claim all 50 tokens.
2. Bob sees this, and Frontruns here with a transaction to claim only the token with ID=49 --> This will result in Alice´s transaction to be reverted in the last iteration. Therefore Alice already spent a great amount of gas and has only minted 1 Token so far.

Because there is a limited Minting period. Bob is repeating this process every time, Alice creates a new transaction for the rest of her tokens. This might result in Alice not being able to claim all her Tokens until the end of the Mint period.

Impact

• Users might not be able to claim all their reserveTokens in time.
• Users have to spend more Gas

Code Snippet

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/minters/MerkleReserveMinter.sol#L129-L173

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/minters/MerkleReserveMinter.sol#L165

Tool used

Manual Review

Recommendation

Do not revert the whole transaction in case a single mint in the loop fails.

[nevillehuang]

Invalid, specific merkle proof required for each token minting based on this check here so this wouldn't be allowed

[Arabadzhiew]

Escalate

This report seems to describe a legit vulnerability. Minters who send a transaction to mint all of their allocated tokens, can get frontran with another transaction that mints just one of those tokens - leading to the original transaction being reverted. This is possible, because after the first transaction gets sent to the mempool, all Merkle claims that it contains will be publicly visible.

In some specific cases, this can be used to prevent minters from receiving all of their allocated tokens.

[sherlock-admin2]

Escalate

This report seems to describe a legit vulnerability. Minters who send a transaction to mint all of their allocated tokens, can get frontran with another transaction that mints just one of those tokens - leading to the original transaction being reverted. This is possible, because after the first transaction gets sent to the mempool, all Merkle claims that it contains will be publicly visible.

In some specific cases, this can be used to prevent minters from receiving all of their allocated tokens.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[nevillehuang]

@Czar102 @neokry @IAm0x52 Is front running merkle proofs possible? If yes this finding might be valid, but I also see no fix for this other than flash-bots?

If I understand correctly, when employing a Merkle proof system, the new proof is computed at a designated moment. If a user fails to claim during the generation of the proof, they essentially have a window of time between when the proof is generated and when it is made published to make their initial claim.

Also, if this is true, the user front-running would still need to pay to mint from reserves so there isn't really a "loss" here.

[Oot2k]

I agree with the escalation:

The admin / frontend generates these proofs all at ones. Which means that anyone can mint one token to the Receiver.

The worst case scenario is:

Mint is going to end soon, user has still not minted his tokens and token price is free (because of DAO migration for example). User gets frontrun and transaction gets reverted, mint ends and user only gets one of his tokens.

This is enough to make this an medium severity finding.

See following past issue for reference: https://solodit.xyz/issues/m-4-full-inventory-asset-purchases-can-be-dosd-via-frontrunning-sherlock-none-index-update-git Same idea, reported a couple month ago and validated as medium.

How can this be prevented? Make sure only the Receiver can mint his tokens. Or as the issue states, do not revert the whole transaction

[neokry]

The loss of funds here is a bit of a stretch as If a user is minting say 5 tokens. this attack can be performed at most 5 times as every time the attacker is executing this the user has one less token to mint and as @nevillehuang mentioned the attacker will be paying for the users tokens if they have a price.

[Arabadzhiew]

The loss of funds here is a bit of a stretch as If a user is minting say 5 tokens. this attack can be performed at most 5 times as every time the attacker is executing this the user has one less token to mint and as @nevillehuang mentioned the attacker will be paying for the users tokens if they have a price.

What if the user, for some reason happened to try and mint their tokens at the last minute? If there are no minting fees, a malicious actor can frontrun their transaction in the way described above and if they do not react quickly to that, they can easily end up not being able to mint all of their allocated tokens.

[nevillehuang]

I think this is a valid medium severity issue, given this can easily be solved by implementing a whitelist and a check within the mintFromReserve() function.

Just to confirm @Oot2k @Shogoki, are there duplicates to your issue? If not I will try to double check.

[Oot2k]

@nevillehuang I don't think so, at least I did not find any.

I partially agree with sponsors comment, but this still makes this a medium based of sherlock rules.

[Shogoki]

I think this is a valid medium severity issue, given this can easily be solved by implementing a whitelist and a check within the mintFromReserve() function.

Just to confirm @Oot2k @Shogoki, are there duplicates to your issue? If not I will try to double check.

Yep, i did also not find a duplicate, while judging.

I think a reasonable fix for this is, like we stated in the report, to not revert the whole transaction in case a single mint fails.

[nevillehuang]

I am revisiting this issue for some clarification as now I am not quite sure about the validity of the issue just yet.

My inquiry is on the following check here

  if (!MerkleProof.verify(claim.merkleProof, settings.merkleRoot, keccak256(abi.encode(claim.mintTo, claim.tokenId)))) {
      revert INVALID_MERKLE_PROOF(claim.mintTo, claim.merkleProof, settings.merkleRoot);
  }

My understanding is front-running is not possible given the merkle proof set by admin offchain to user minting to reserve must correspond to the correct leaf parameters claim.mintTo() and claim.tokenId. So if the merkle proof assigned to Bob does not correspond to the leaf inputs (mintTo and tokenId) inputted by Bob in the above scenario doesn't it mean it will revert i.e. Doesn't that mean if Bob front-runs and hijack the claim.merkleProof assigned to Alice, he will still need to mint token with tokenId to mintTo controlled by Alice instead of his own address if not it will revert?

I think my query can be clarified by a coded PoC, if the above is untrue, then I think this should be a medium severity issue, if not this should be invalid.

[Oot2k]

We don't steal tokens from bob. We just mint 1 token to bob and make his "mint 100 tokens" transaction revert.

The user, in your example bob, can mint tokens by providing an array of proofs, the loop will check if these leafs are valid, and then mint tokens.

The issue now is that alice can mint tokens for bob and make his mint transaction revert. In case the mint reserve time is running out, and bob calls mint to mint his 100 tokens, alice can frontrun to mint 1 of these 100 tokens.

Bobs transaction tires to mint this one token minted prior by alice, this will revert. Time will run out and bob is left with only 1 of his 100 tokens.

The revert is inside of the mint because the token is already minted. The claims in this case are user provided and not checked in any way, so anyone is allowed to mint in behalf of others.

Even if this attack is unlikely it should still be considered medium. I think medium is meant exactly for these issues.

I hope this clarifies the issue. Also the core problem is really similar to the issue I linked in an above comment.

[Czar102]

What if the user, for some reason happened to try and mint their tokens at the last minute?

The user should have enough time to get their transaction properly included (execution success) in the block, if they are risking not having enough time, it's their fault.

Anyone can always mint tokens one by one, and if anyone else claimed their tokens for them, they are already paid for!

Essentially, minting is not impacted if you are minting one by one, the potential UX issues arise when claiming many tokens. Claiming many tokens at once does not impact core functionality (as you can still mint tokens one by one) and hence I consider this issue a low severity one.

[Oot2k]

I respect sherlock final decision but still want to quote the docs (how to identify a medium issue):

"Causes a loss of funds but requires certain external conditions or specific states."

-> it requires specific state, but causes loss

Also consider following case: per mint is only open for short amount of time ( < 30min) to create fomo, in this case this attack can be much more realistic. Minting tokens 1 by 1 should be considered another case / a different "feature". In my provided example of small mint window its not recommended to mint tokens 1 by 1 if there is a large amount to claim.

I hope these points clarify the issue further.

[nevillehuang]

@Oot2k are you saying the tokenId and mintTo inputted by Bob won't be verified against the merkle proof provided for him by the admin? I.e. Will he only be able to mint the token with specific tokenId (e.g. 49, provided within the MerkleClaim[] claims array ) and to a specific mintTo address?

That is what I am unsure about, so I think unless proven I'm not sure about the validity of this issue.

[Shogoki]

@Oot2k are you saying the tokenId and mintTo inputted by Bob won't be verified against the merkle proof provided for him by the admin? I.e. Will he only be able to mint the token with specific tokenId (e.g. 49, provided within the MerkleClaim[] claims array ) and to a specific mintTo address?

That is what I am unsure about, so I think unless proven I'm not sure about the validity of this issue.

Alice is providing multiple proofs, including the tokenId and the addressTo, to mint multiple (all her) tokens. Bob can see all these data in the mempool, and take only one of them to mint only this one token for her, which will the whole transaction of Alice revert.

Regarding validity

I believe it is a main feature for the user to mint multiple (all his) tokens at once, as it makes sense also from a gas perspective.

• There is not necessarily a price for minting set, so cost for Bob might be only gas.
• Minting is time limited, yes the user should have enough time to get his TXin the block. But he would not expect multiple reverts

In regards to this I believe there is a valid chance, that this could happen and Bob can make Alice to miss out on some of her Tokens —> loss of funds.

So therefore, and what @oot2k said, i think this should be a valid Medium.

[Czar102]

I believe it is a main feature for the user to mint multiple (all his) tokens at once, as it makes sense also from a gas perspective.

The core functionality is minting tokens and minting multiple tokens at once seems to simply be an optimization. (NOT core protocol functionality)

I think some arguments also implicitly speculate on how the frontend will work, which is out of scope.

I see the time restriction, but I don't think it's a significant enough argument to validate the issue. For most cases this should be a reasonable timeframe (maybe at least 24h to be fair with respect to all timezones) and creating these short fomo claims is positively correlated with the minting fee (why would you fomo if there is no way to monetize it?).

I still think that when a window exists, making a transaction shouldn't be so time-sensitive. I stand by my initial judgment that this should be a low severity issue.

[Arabadzhiew]

I believe since this functionality is implemented, it is reasonable to assume that users are going to rely on it. And they for sure won't be expecting their valid transactions to get reverted.

One scenario that comes to my mind where this will potentially be a problem is a period of high gas prices. In such a period, some users will likely wait around until sending their transactions, potentially until the last moment, as to try and catch the lowest gas price possible. In that case, if they aren't aware of the vulnerability at question, it can indeed be used to prevent them from claiming all of their tokens, since they are going to rely on the multiple mint functionality, despite it not being a core protocol functionality.

Anyway, that's my two cents.

[Czar102]

Please note the number of assumptions that are being made. You also can't assume that the interface will have no way of purchasing the tokens one by one.

[Czar102]

Result: Low Unique

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• Arabadzhiew: rejected

[Czar102]

Advising sponsors to fix this issue despite being a low severity one for UX reasons.

[neokry]

Fixed here: https://github.com/ourzora/nouns-protocol/pull/127

[IAm0x52]

Fix looks good. It is now impossible to DOS free mints. Paid mints can still be DOS'd but as discussed above the attacker would have to pay to mint the token.