overflow checks

config/default.js

@@ -86,7 +86,11 @@ module.exports = {
   USE_STUBS: process.env.USE_STUBS === 'true',
   VK_IDS: { deposit: 0, single_transfer: 1, double_transfer: 2, withdraw: 3 }, // used as an enum to mirror the Shield contracts enum for vk types. The keys of this object must correspond to a 'folderpath' (the .zok file without the '.zok' bit)
   TIMBER_HEIGHT: 32,
-
+  MAX_PUBLIC_VALUES: {
+    ERCADDRESS: 2n ** 161n - 1n,
+    COMMITMENT: 2n ** 249n - 1n,
+    NULLIFIER: 2n ** 249n - 1n,
+  },
   // the various parameters needed to describe the Babyjubjub curve that we use for El-Gamal
   // BABYJUBJUB
   // Montgomery EC form is y^2 = x^3 + Ax^2 + Bx

nightfall-client/src/classes/commitment.mjs

@@ -31,13 +31,17 @@ class Commitment {
       compressedPkd,
       salt,
     });
-    this.hash = sha256([
-      this.preimage.ercAddress,
-      this.preimage.tokenId,
-      this.preimage.value,
-      this.preimage.compressedPkd,
-      this.preimage.salt,
-    ]);
+    // we truncate the hash down to 31 bytes but store it in a 32 byte variable
+    // this is consistent to what we do in the ZKP circuits
+    this.hash = generalise(
+      sha256([
+        this.preimage.ercAddress,
+        this.preimage.tokenId,
+        this.preimage.value,
+        this.preimage.compressedPkd,
+        this.preimage.salt,
+      ]).hex(32, 31),
+    );
   }
 
   // sometimes (e.g. going over http) the general-number class is inconvenient

nightfall-client/src/classes/nullifier.mjs

@@ -16,7 +16,9 @@ class Nullifier {
       nsk,
       commitment: commitment.hash,
     });
-    this.hash = sha256([this.preimage.nsk, this.preimage.commitment]);
+    // we truncate the hash down to 31 bytes but store it in a 32 byte variable
+    // this is consistent to what we do in the ZKP circuits
+    this.hash = generalise(sha256([this.preimage.nsk, this.preimage.commitment]).hex(32, 31));
   }
 }
 

nightfall-client/src/services/transfer.mjs

@@ -108,7 +108,6 @@ async function transfer(transferParams) {
 
   // compress the secrets to save gas
   const compressedSecrets = Secrets.compressSecrets(secrets);
-
   const commitmentTreeInfo = await Promise.all(oldCommitments.map(c => getSiblingInfo(c)));
   const localSiblingPaths = commitmentTreeInfo.map(l => {
     const path = l.siblingPath.path.map(p => p.value);
@@ -166,7 +165,13 @@ async function transfer(transferParams) {
       secrets.cipherText.flat().map(text => text.field(BN128_GROUP_ORDER)),
       ...secrets.squareRootsElligator2.map(sqroot => sqroot.field(BN128_GROUP_ORDER)),
     ],
-    compressedSecrets.map(text => generalise(text.hex(32, 31)).field(BN128_GROUP_ORDER)),
+    compressedSecrets.map(text => {
+      const bin = text.binary.padStart(256, '0');
+      const parity = bin[0];
+      const ordinate = bin.slice(1);
+      const fields = [parity, new GN(ordinate, 'binary').field(BN128_GROUP_ORDER)];
+      return fields;
+    }),
   ].flat(Infinity);
 
   logger.debug(`witness input is ${witness.join(' ')}`);

nightfall-deployer/circuits/double_transfer.zok

@@ -42,6 +42,11 @@ struct Secrets {
 	field sqrtMessage4
 }
 
+struct CompressedPoint {
+	bool parity
+	field ordinate
+}
+
 def main(\
 	private field[2] fErcAddress,\
 	private OldCommitmentPreimage[2] oldCommitment,\
@@ -53,7 +58,7 @@ def main(\
 	private field[2][32] path,\
 	private field[2] order,\
 	private Secrets secrets,\
-	field[8] compressedCipherText\
+	CompressedPoint[8] compressedCipherText\
 )->():
 
 	BabyJubJubParams context = curveParams()
@@ -155,7 +160,6 @@ def main(\
 	assert(u32_to_bool_32(sha[0])[8..32] == u32_to_bool_32(newCommitmentHash[1][0])[8..32])
 
 	// check the old commitments are valid
-	// old commitments are private inputs, so they are u32[8] and not truncated
 	for u32 i in 0..2 do
 		sha = sha256of1280([\
 			...ercAddress[i],\
@@ -164,7 +168,11 @@ def main(\
 			...pkdU32,\
 			...oldCommitment[i].salt\
 		])
-		assert(sha == oldCommitment[i].hash)
+		// assert(sha == oldCommitment[i].hash)
+		// last 224 bits:
+		assert(sha[1..8] == oldCommitment[i].hash[1..8])
+		// first 24 bits:
+		assert(u32_to_bool_32(sha[0])[8..32] == u32_to_bool_32(oldCommitment[i].hash[0])[8..32])
 	endfor
 
 	// And the encryption of the transaction (extend the value up to 256 bits)
@@ -178,10 +186,10 @@ def main(\
 		// there is likely a compiler bug with zokrates 0.6.4 which makes using spreads (e.g. [8..256]) inside a function (e.g. assert()) very slow
 		u32 j = 2*i
 		bool[256] compressed256 = edwardsCompress([secrets.cipherText[j], secrets.cipherText[j+1]])
-		bool[256] compressedCheck256 = field_to_bool_256(compressedCipherText[i])
-		bool[248] compressed = compressed256[8..256]
-		bool[248] compressedCheck = compressedCheck256[8..256]
-		assert(compressed == compressedCheck)
+		bool parity = compressedCipherText[i].parity
+		bool[256] ordinate = field_to_bool_256(compressedCipherText[i].ordinate)
+		bool[256] compressedCheck256 = [ parity, ...ordinate[1..256] ]
+		assert(compressed256 == compressedCheck256)
 	endfor
 
 	// check that the old commitments are in the merkle tree

nightfall-deployer/circuits/double_transfer_stub.zok

@@ -25,6 +25,11 @@ struct Secrets {
 	field sqrtMessage4
 }
 
+struct CompressedPoint {
+	bool parity
+	field ordinate
+}
+
 def main(\
 	private field[2] fErcAddress,\
 	private OldCommitmentPreimage[2] oldCommitment,\
@@ -36,11 +41,12 @@ def main(\
 	private field[2][32] path,\
 	private field[2] order,\
 	private Secrets secrets,\
-	field[8] compressedCipherText\
+	CompressedPoint[8] compressedCipherText\
 )->():
 
 	field u = 0
 	u32 v = 0x00000000
+	bool b = true
 	for u32 j in 0..2 do
 		u = u + fErcAddress[j] + fNewCommitmentHash[j] + fNullifier[j] - root[j]
 		for u32 i in 0..8 do
@@ -56,7 +62,8 @@ def main(\
 
 	u32 w = 0x00000000
 	for u32 i in 0..8 do
-		u = u + compressedCipherText[i]
+		u = u + compressedCipherText[i].ordinate
+		b = b && compressedCipherText[i].parity
 		w = w + secrets.ephemeralKey1[i] +\
 			secrets.ephemeralKey2[i] +\
 			secrets.ephemeralKey3[i] +\
@@ -83,5 +90,6 @@ def main(\
 	assert(v == v)
 	assert(u == u)
 	assert(w == w)
+	assert(b == b)
 
 	return

nightfall-deployer/circuits/single_transfer.zok

@@ -42,6 +42,11 @@ struct Secrets {
 	field sqrtMessage4
 }
 
+struct CompressedPoint {
+	bool parity
+	field ordinate
+}
+
 def main(\
 	private field fErcAddress,\
 	private OldCommitmentPreimage oldCommitment,\
@@ -53,7 +58,7 @@ def main(\
 	private field[32] path,\
 	private field order,\
 	private Secrets secrets,\
-	field[8] compressedCipherText\
+	CompressedPoint[8] compressedCipherText\
 )->():
 
 	BabyJubJubParams context = curveParams()
@@ -110,15 +115,18 @@ def main(\
 	assert(u32_to_bool_32(sha[0])[8..32] == u32_to_bool_32(newCommitmentHash[0])[8..32])
 
 	// check the old commitment is valid
-	// old commitments are private inputs, so they are u32[8] and not truncated
 	sha = sha256of1280([\
 		...ercAddress,\
 		...oldCommitment.id,\
 		...oldCommitment.value,\
 		...pkdU32,\
 		...oldCommitment.salt\
 	])
-	assert(sha == oldCommitment.hash)
+	// assert(sha == oldCommitment.hash)
+	// last 224 bits:
+	assert(sha[1..8] == oldCommitment.hash[1..8])
+	// first 24 bits:
+	assert(u32_to_bool_32(sha[0])[8..32] == u32_to_bool_32(oldCommitment.hash[0])[8..32])
 
 	// And the encryption of the transaction (extend the value up to 256 bits)
 	assert(secrets.cipherText == enc4(ercAddress, oldCommitment.id, oldCommitment.value, newCommitment.salt, newCommitment.pkdRecipient, secrets.ephemeralKey1, secrets.ephemeralKey2, secrets.ephemeralKey3, secrets.ephemeralKey4, secrets.sqrtMessage1, secrets.sqrtMessage2, secrets.sqrtMessage3, secrets.sqrtMessage4))
@@ -130,10 +138,10 @@ def main(\
 		// there is likely a compiler bug with zokrates 0.6.4 which makes using spreads (e.g. [8..256]) inside a function (e.g. assert()) very slow
 		u32 j = 2*i
 		bool[256] compressed256 = edwardsCompress([secrets.cipherText[j], secrets.cipherText[j+1]])
-		bool[256] compressedCheck256 = field_to_bool_256(compressedCipherText[i])
-		bool[248] compressed = compressed256[8..256]
-		bool[248] compressedCheck = compressedCheck256[8..256]
-		assert(compressed == compressedCheck)
+		bool parity = compressedCipherText[i].parity
+		bool[256] ordinate = field_to_bool_256(compressedCipherText[i].ordinate)
+		bool[256] compressedCheck256 = [ parity, ...ordinate[1..256] ]
+		assert(compressed256 == compressedCheck256)
 	endfor
 
 	// check that the old commitment is in the merkle tree (path[0] should be the root)

nightfall-deployer/circuits/single_transfer_stub.zok

@@ -24,6 +24,11 @@ struct Secrets {
 	field sqrtMessage4
 }
 
+struct CompressedPoint {
+	bool parity
+	field ordinate
+}
+
 def main(\
 	private field fErcAddress,\
 	private OldCommitmentPreimage oldCommitment,\
@@ -35,11 +40,12 @@ def main(\
 	private field[32] path,\
 	private field order,\
 	private Secrets secrets,\
-	field[8] compressedCipherText\
+	CompressedPoint[8] compressedCipherText\
 )->():
 
 	field u = fErcAddress + fNewCommitmentHash + fNullifier - root
 	u32 v = 0x00000000
+	bool b = true
 	for u32 i in 0..8 do
 		v = v + oldCommitment.id[i] +\
 			oldCommitment.value[i] +\
@@ -52,7 +58,8 @@ def main(\
 
 	u32 w = 0x00000000
 	for u32 i in 0..8 do
-		u = u + compressedCipherText[i]
+		u = u + compressedCipherText[i].ordinate
+		b = b && compressedCipherText[i].parity
 		w = w + secrets.ephemeralKey1[i] +\
 			secrets.ephemeralKey2[i] +\
 			secrets.ephemeralKey3[i] +\
@@ -71,5 +78,6 @@ def main(\
 	assert(v == v)
 	assert(u == u)
 	assert(w == w)
+	assert(b == b)
 
 	return

nightfall-deployer/circuits/withdraw.zok

@@ -65,14 +65,17 @@ def main(\
 	assert(u32_to_bool_32(sha[0])[8..32] == u32_to_bool_32(nullifier[0])[8..32])
 
 	// check the old commitment is valid
-	// old commitments are private inputs, so they are u32[8] and not truncated
-	assert(oldCommitment.hash == sha256of1280([\
+	sha = sha256of1280([\
 		...ercAddress,\
 		...id,\
 		...value,\
 		...pkdU32,\
 		...oldCommitment.salt\
-	]))
+	])
+	// last 224 bits:
+	assert(sha[1..8] == oldCommitment.hash[1..8])
+	// first 24 bits:
+	assert(u32_to_bool_32(sha[0])[8..32] == u32_to_bool_32(oldCommitment.hash[0])[8..32])
 
 	// check that the old commitment is in the merkle tree
 	field mimcHash = u32_8_to_field(oldCommitment.hash)

nightfall-deployer/contracts/Challenges.sol

@@ -151,6 +151,11 @@ contract Challenges is Stateful, Key_Registry, Config {
     ) external onlyBootChallenger {
         checkCommit(msg.data);
         state.isBlockReal(blockL2, transactions, blockNumberL2);
+        // first check the transaction and block do not overflow
+        ChallengesUtil.libCheckOverflows(
+          blockL2,
+          transactions[transactionIndex]
+        );
         // now we need to check that the proof is correct
         ChallengesUtil.libCheckCompressedProof(
             transactions[transactionIndex].proof,
@@ -189,6 +194,11 @@ contract Challenges is Stateful, Key_Registry, Config {
             transactions[transactionIndex].historicRootBlockNumberL2[0] ==
                 blockNumberL2ContainingHistoricRoot
         );
+        // first check the transaction and block do not overflow
+        ChallengesUtil.libCheckOverflows(
+          blockL2,
+          transactions[transactionIndex]
+        );
         // now we need to check that the proof is correct
         ChallengesUtil.libCheckCompressedProof(
             transactions[transactionIndex].proof,
@@ -236,6 +246,11 @@ contract Challenges is Stateful, Key_Registry, Config {
                 blockNumberL2ContainingHistoricRoot[1],
             'Incorrect historic root block'
         );
+        // first check the transaction and block do not overflow
+        ChallengesUtil.libCheckOverflows(
+          blockL2,
+          transactions[transactionIndex]
+        );
         // now we need to check that the proof is correct
         ChallengesUtil.libChallengeProofVerification(
             transactions[transactionIndex],

nightfall-deployer/contracts/ChallengesUtil.sol

@@ -10,6 +10,9 @@ import './Structures.sol';
 library ChallengesUtil {
     bytes32 public constant ZERO =
         0x0000000000000000000000000000000000000000000000000000000000000000;
+    uint256 public constant MAX31 = 2 ** 249 - 1;
+    uint256 public constant MAX20 = 2 ** 161 - 1;
+    uint256 public constant BN128_GROUP_ORDER = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
 
     function libChallengeLeafCountCorrect(
         Structures.Block memory priorBlockL2,
@@ -203,6 +206,19 @@ library ChallengesUtil {
         );
     }
 
+    function libCheckOverflows(
+      Structures.Block calldata blockL2,
+      Structures.Transaction calldata transaction
+    ) public pure {
+      require(uint256(transaction.ercAddress) <= MAX20, 'ERC address out of range');
+      require(uint256(transaction.recipientAddress) <= MAX20, 'Recipient ERC address out of range');
+      require(uint256(transaction.commitments[0]) <= MAX31, 'Commitment 0 out of range');
+      require(uint256(transaction.commitments[1]) <= MAX31, 'Commitment 1 out of range');
+      require(uint256(transaction.nullifiers[0]) <= MAX31, 'Nullifier 0 out of range');
+      require(uint256(transaction.nullifiers[1]) <= MAX31, 'Nullifier 1 out of range');
+      require(uint256(blockL2.root) < BN128_GROUP_ORDER, 'root out of range');
+    }
+
     function libChallengeNullifier(
         Structures.Transaction memory tx1,
         uint256 nullifierIndex1,

nightfall-deployer/contracts/State.sol

@@ -80,6 +80,8 @@ contract State is Structures, Initializable, ReentrancyGuardUpgradeable, Config
             ); // this will fail if a tx is re-mined out of order due to a chain reorg.
         require(BLOCK_STAKE <= msg.value, 'The stake payment is incorrect');
         require(b.proposer == msg.sender, 'The proposer address is not the sender');
+        // set the maximum tx/block to prevent unchallengably large blocks
+        require (t.length < 33, 'The block has too many transactions');
         // We need to set the blockHash on chain here, because there is no way to
         // convince a challenge function of the (in)correctness by an offchain
         // computation; the on-chain code doesn't save the pre-image of the hash so

nightfall-deployer/contracts/Verifier.sol

@@ -34,6 +34,8 @@ library Verifier {
 
   using Pairing for *;
 
+  uint256 constant BN128_GROUP_ORDER = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
+
   struct Proof_G16 {
       Pairing.G1Point A;
       Pairing.G2Point B;
@@ -98,7 +100,9 @@ library Verifier {
           // The following success variables replace require statements with corresponding functions called. Removing require statements to ensure a wrong proof verification challenge's require statement correctly works
           bool success_sm_qpih;
           bool success_vkdi_sm_qpih;
-          for (uint i = 0; i < _publicInputs.length; i++) {
+            for (uint i = 0; i < _publicInputs.length; i++) {
+            // check for overflow attacks
+            if (_publicInputs[i] >= BN128_GROUP_ORDER) return 2;
             (sm_qpih, success_sm_qpih) = Pairing.scalar_mul(vk.gamma_abc[i+1], _publicInputs[i]);
             (vk_dot_inputs, success_vkdi_sm_qpih) = Pairing.addition(
               vk_dot_inputs,