Merkle proof implementation is subject to tree extension attacks

[Austin-Williams]

The merkle proof code does not check the length of the proofs, and so it is susceptible to a tree-extension attack (or second-preimage attack).

While this same code has been used in other projects, the key difference here is that the leaf nodes (recipient addresses) have been chosen by wardens (potential adversaries), whereas in other projects the leaf nodes are addresses grabbed from historical blockchain data (not chosen by potential adversaries).

This means a malicious airdrop receiver could have given you a malicious bytes32 "address" which is actually its own merkle root for a tree of addresses that the attacker controls. If you used such a malicious address as a leaf in your merkle tree, it would allow the attacker to generate arbitrarily many valid proofs for addresses they control.

After computing your merkle tree and learning its depth -- and before deploying these contracts -- I recommend adding a require(merkleProof.length == treeDepth, 'invalid proof') just above this line.

[MrToph]

While this same code has been used in other projects, the key difference here is that the leaf nodes (recipient addresses) have been chosen by wardens (potential adversaries)

Good point but the leaf nodes are keccak(recipient, amount) where the claim amount has been chosen by the C4 team, does this prevent this?

If I understand your attack correctly, the attacker would do:

• h1 = keccak(attacker1, uint256.max)
• h2 = keccak(attacker2, uint256.max)
• h = keccak(h1, h2) and then choose and submit a recipient address (for airdrop inclusion) such that h == keccak(recipient, amount) where amount was chosen by the C4 team. They'd need to find a second preimage (recipient, amount) != (h1, h2) which goes against the collision resistance of keccak?

[Austin-Williams]

Ah okay! I was under the impression that the user-provided addresses were being used (directly) as leaves of the merkle tree.

As long as the leaves themselves aren't attacker-chosen, then that's fine. (It's even fine that they are attacker influenced in the way you mention above).

I think we're okay.

[Austin-Williams]

Okay, I'm sufficiently satisfied that an attacker could not get a "bad merkle root" as a leaf in the tree via the form submission.