Merge pull request #2 from kobigurk/fix/nullifier-exploit

merge kobigurk/fix: don't allow double-spending with a large nullifier

Author: sragss

contracts/PrivateAirdrop.sol

@@ -20,6 +20,8 @@ contract PrivateAirdrop is Ownable {
 
     mapping(bytes32 => bool) public nullifierSpent;
 
+    uint256 constant SNARK_FIELD = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
+
     constructor(
         IERC20 _airdropToken,
         uint _amountPerRedemption,
@@ -34,6 +36,7 @@ contract PrivateAirdrop is Ownable {
 
     /// @notice verifies the proof, collects the airdrop if valid, and prevents this proof from working again.
     function collectAirdrop(bytes calldata proof, bytes32 nullifierHash) public {
+        require(uint256(nullifierHash) < SNARK_FIELD ,"Nullifier is not within the field");
         require(!nullifierSpent[nullifierHash], "Airdrop already redeemed");
 
         uint[] memory pubSignals = new uint[](3);
@@ -51,4 +54,4 @@ contract PrivateAirdrop is Ownable {
     function updateRoot(bytes32 newRoot) public onlyOwner {
         root = newRoot;
     }
-}
+}

test/PrivateAirdropIntegration.spec.ts

@@ -47,13 +47,53 @@ describe("PrivateAirdrop", async () => {
 
         // Collect
         let keyHash = toHex(pedersenHash(key))
+
         let execute = await (
             await airdrop.connect(redeemer).collectAirdrop(callData, keyHash)).wait()
         expect(execute.status).to.be.eq(1)
         let contractBalanceUpdated: BigNumber = await erc20.balanceOf(airdrop.address);
         expect(contractBalanceUpdated.toNumber()).to.be.eq(contractBalanceInit.toNumber() - NUM_ERC20_PER_REDEMPTION)
         let redeemerBalance: BigNumber = await erc20.balanceOf(redeemer.address);
         expect(redeemerBalance.toNumber()).to.be.eq(NUM_ERC20_PER_REDEMPTION)
+
+    })
+
+    it("cannot exploit using public inputs larger than the scalar field", async () => {
+        // Deploy contracts
+        let hexRoot = toHex(merkleTreeAndSource.merkleTree.root.val)
+        let [universalOwnerSigner, erc20SupplyHolder, redeemer] = await ethers.getSigners();
+        let {erc20, verifier, airdrop} = 
+            await deployContracts(
+                universalOwnerSigner, 
+                erc20SupplyHolder.address, 
+                hexRoot);
+
+        // Transfer airdroppable tokens to contract
+        await erc20.connect(erc20SupplyHolder).transfer(airdrop.address, NUM_ERC20_TO_DISTRIBUTE);
+        let contractBalanceInit: BigNumber = await erc20.balanceOf(airdrop.address);
+        expect(contractBalanceInit.toNumber()).to.be.eq(NUM_ERC20_TO_DISTRIBUTE);
+
+        let merkleTree = merkleTreeAndSource.merkleTree; 
+
+        // Generate proof
+        let leafIndex = 7;
+        let key = merkleTreeAndSource.leafNullifiers[leafIndex];
+        let secret = merkleTreeAndSource.leafSecrets[leafIndex];
+        let callData = await generateProofCallData(merkleTree, key, secret, redeemer.address, WASM_BUFF, ZKEY_BUFF);
+
+        // Collect
+        let keyHash = toHex(pedersenHash(key))
+	let keyHashTwo = toHex(BigInt(keyHash) + BigInt('21888242871839275222246405745257275088548364400416034343698204186575808495617'))
+
+        let execute = await (
+            await airdrop.connect(redeemer).collectAirdrop(callData, keyHash)).wait()
+        expect(execute.status).to.be.eq(1)
+        let contractBalanceUpdated: BigNumber = await erc20.balanceOf(airdrop.address);
+        expect(contractBalanceUpdated.toNumber()).to.be.eq(contractBalanceInit.toNumber() - NUM_ERC20_PER_REDEMPTION)
+        let redeemerBalance: BigNumber = await erc20.balanceOf(redeemer.address);
+        expect(redeemerBalance.toNumber()).to.be.eq(NUM_ERC20_PER_REDEMPTION)
+        await expect(airdrop.connect(redeemer).collectAirdrop(callData, keyHashTwo)).to.be.revertedWith("Nullifier is not within the field")
+
     })
 
     it("cannot be front-run by another party", async () => {
@@ -176,4 +216,4 @@ async function deployContracts(
                 root
                 )) as PrivateAirdrop
         return {erc20, verifier, airdrop}
-}
+}