veridise audit fix: First header value chars not validated

README.md

@@ -17,7 +17,6 @@ The library exports the following functions:
 - `headers::constrain_header_field` - constrain an index/ length in the header to be the correct name, full, and uninterrupted
 - `partial_hash::partial_sha256_var_end` - finish a precomputed sha256 hash over the body
 - `masking::mask_text` - apply a byte mask to the header or body to selectively reveal parts of the entire email
-- `standard_outputs` - returns the hash of the DKIM pubkey and a nullifier for the email (`hash(signature)`)
 
 Additionally, the `@zk-email/zkemail-nr` JS library exports an ergonomic API for easily deriving circuit inputs needed to utilize the Noir library.
 
@@ -28,9 +27,9 @@ A basic email verifier will often look like this:
 ```rust
 use dep::zkemail::{
     KEY_LIMBS_1024, dkim::RSAPubkey, get_body_hash_by_index,     
-    base64::body_hash_base64_decode, standard_outputs
+    base64::body_hash_base64_decode
 };
-use dep::std::hash::sha256_var;
+use dep::std::hash::{sha256_var, pedersen_hash};
 
 // Somewhere in your function
 ...

examples/email_mask/src/main.nr

@@ -1,8 +1,8 @@
 use dep::zkemail::{
     KEY_LIMBS_2048, dkim::RSAPubkey, headers::body_hash::get_body_hash,
-    standard_outputs, Sequence, masking::mask_text
+    Sequence, masking::mask_text
 };
-use dep::std::{collections::bounded_vec::BoundedVec, hash::sha256_var};
+use dep::std::{collections::bounded_vec::BoundedVec, hash::{pedersen_hash, sha256_var}};
 
 global MAX_EMAIL_HEADER_LENGTH: u32 = 512;
 global MAX_EMAIL_BODY_LENGTH: u32 = 1024;
@@ -55,6 +55,7 @@ fn main(
     let masked_body = mask_text(body, body_mask);
 
     // hash the pubkey and signature for the standard outputs
-    let standard_out = standard_outputs(pubkey.modulus, signature);
+    let email_nullifier = pedersen_hash(signature);
+    let standard_out = [pubkey.hash(), email_nullifier];
     (standard_out, masked_header, masked_body)
 }

examples/extract_addresses/src/main.nr

@@ -1,12 +1,10 @@
 use dep::zkemail::{
-    KEY_LIMBS_2048, dkim::RSAPubkey,
-    headers::{body_hash::get_body_hash, email_address::get_email_address}, standard_outputs, Sequence,
-    MAX_EMAIL_ADDRESS_LENGTH
+    KEY_LIMBS_2048, dkim::RSAPubkey, headers::email_address::get_email_address, Sequence,
+    MAX_EMAIL_ADDRESS_LENGTH,
 };
-use dep::std::{collections::bounded_vec::BoundedVec, hash::sha256_var};
+use dep::std::{collections::bounded_vec::BoundedVec, hash::pedersen_hash};
 
 global MAX_EMAIL_HEADER_LENGTH: u32 = 512;
-global MAX_EMAIL_BODY_LENGTH: u32 = 1024;
 
 /**
  * Verify an arbitrary email signed by a 2048-bit RSA DKIM signature and extract sender and recipient addresses
@@ -30,26 +28,23 @@ fn main(
     from_header_sequence: Sequence,
     from_address_sequence: Sequence,
     to_header_sequence: Sequence,
-    to_address_sequence: Sequence
-) -> pub ([Field; 2], BoundedVec<u8, MAX_EMAIL_ADDRESS_LENGTH>, BoundedVec<u8, MAX_EMAIL_ADDRESS_LENGTH>) {
+    to_address_sequence: Sequence,
+    ) -> pub ([Field; 2], BoundedVec<u8, MAX_EMAIL_ADDRESS_LENGTH>, BoundedVec<u8, MAX_EMAIL_ADDRESS_LENGTH>) {
     // check the body and header lengths are within bounds
     assert(header.len() <= MAX_EMAIL_HEADER_LENGTH);
 
     // verify the dkim signature over the header
     pubkey.verify_dkim_signature(header, signature);
 
     // extract to and from email addresses
-    let from = comptime {
-        "from".as_bytes()
-    };
-    let to = comptime {
-        "to".as_bytes()
-    };
+    let from = comptime { "from".as_bytes() };
+    let to = comptime { "to".as_bytes() };
     // 16k gate cost? has to be able to be brought down
     let from_address = get_email_address(header, from_header_sequence, from_address_sequence, from);
     let to_address = get_email_address(header, to_header_sequence, to_address_sequence, to);
 
     // hash the pubkey and signature for the standard outputs
-    let standard_out = standard_outputs(pubkey.modulus, signature);
+    let email_nullifier = pedersen_hash(signature);
+    let standard_out = [pubkey.hash(), email_nullifier];
     (standard_out, from_address, to_address)
 }

examples/partial_hash/src/main.nr

@@ -1,7 +1,8 @@
 use dep::zkemail::{
     KEY_LIMBS_2048, dkim::RSAPubkey, headers::body_hash::get_body_hash,
-    partial_hash::partial_sha256_var_end, standard_outputs, Sequence
+    partial_hash::partial_sha256_var_end, Sequence
 };
+use std::hash::pedersen_hash;
 
 global MAX_EMAIL_HEADER_LENGTH: u32 = 512;
 global MAX_PARTIAL_EMAIL_BODY_LENGTH: u32 = 192;
@@ -52,5 +53,6 @@ fn main(
     );
 
     // hash the pubkey and signature for the standard outputs
-    standard_outputs(pubkey.modulus, signature)
+    let email_nullifier = pedersen_hash(signature);
+    [pubkey.hash(), email_nullifier]
 }

examples/remove_soft_line_breaks/src/main.nr

@@ -1,8 +1,8 @@
 use zkemail::{
-    KEY_LIMBS_2048, dkim::RSAPubkey, headers::body_hash::get_body_hash,
-    standard_outputs, Sequence, remove_soft_line_breaks::remove_soft_line_breaks
+    KEY_LIMBS_2048, dkim::RSAPubkey, headers::body_hash::get_body_hash, Sequence,
+    remove_soft_line_breaks::remove_soft_line_breaks,
 };
-use std::hash::sha256_var;
+use std::hash::{pedersen_hash, sha256_var};
 
 global MAX_EMAIL_HEADER_LENGTH: u32 = 512;
 global MAX_EMAIL_BODY_LENGTH: u32 = 1024;
@@ -28,7 +28,7 @@ fn main(
     pubkey: RSAPubkey<KEY_LIMBS_2048>,
     signature: [Field; KEY_LIMBS_2048],
     body_hash_index: u32,
-    dkim_header_sequence: Sequence
+    dkim_header_sequence: Sequence,
 ) -> pub [Field; 2] {
     // check the body and header lengths are within bounds
     assert(header.len() <= MAX_EMAIL_HEADER_LENGTH);
@@ -48,17 +48,19 @@ fn main(
 
     // compare the body hashes
     assert(
-        signed_body_hash == computed_body_hash, "SHA256 hash computed over body does not match body hash found in DKIM-signed header"
+        signed_body_hash == computed_body_hash,
+        "SHA256 hash computed over body does not match body hash found in DKIM-signed header",
     );
 
     // ~ 37,982 constraints
     // ensure the decoded body is the same as the original body
     assert(
         remove_soft_line_breaks(body.storage(), decoded_body.storage()),
-        "Decoded body does not properly remove soft line breaks"
+        "Decoded body does not properly remove soft line breaks",
     );
 
     // ~ 10,255 constraints
     // hash the pubkey and signature for the standard outputs
-    standard_outputs(pubkey.modulus, signature)
+    let email_nullifier = pedersen_hash(signature);
+    [pubkey.hash(), email_nullifier]
 }

examples/verify_email_1024_bit_dkim/src/main.nr

@@ -1,8 +1,8 @@
 use dep::zkemail::{
     KEY_LIMBS_1024, dkim::RSAPubkey, headers::body_hash::get_body_hash,
-    standard_outputs, Sequence
+    Sequence
 };
-use dep::std::{collections::bounded_vec::BoundedVec, hash::sha256_var};
+use dep::std::{collections::bounded_vec::BoundedVec, hash::{sha256_var, pedersen_hash}};
 
 global MAX_EMAIL_HEADER_LENGTH: u32 = 512;
 global MAX_EMAIL_BODY_LENGTH: u32 = 1024;
@@ -48,5 +48,6 @@ fn main(
     );
 
     // hash the pubkey and signature for the standard outputs
-    standard_outputs(pubkey.modulus, signature)
+    let email_nullifier = pedersen_hash(signature);
+    [pubkey.hash(), email_nullifier]
 }

examples/verify_email_2048_bit_dkim/src/main.nr

@@ -1,8 +1,8 @@
 use dep::zkemail::{
     KEY_LIMBS_2048, dkim::RSAPubkey, headers::body_hash::get_body_hash,
-    standard_outputs, Sequence
+    Sequence
 };
-use dep::std::{collections::bounded_vec::BoundedVec, hash::sha256_var};
+use dep::std::{collections::bounded_vec::BoundedVec, hash::{sha256_var, pedersen_hash}};
 
 global MAX_EMAIL_HEADER_LENGTH: u32 = 512;
 global MAX_EMAIL_BODY_LENGTH: u32 = 1024;
@@ -52,5 +52,6 @@ fn main(
 
     // ~ 10,255 constraints
     // hash the pubkey and signature for the standard outputs
-    standard_outputs(pubkey.modulus, signature)
+    let email_nullifier = pedersen_hash(signature);
+    [pubkey.hash(), email_nullifier]
 }

lib/src/dkim.nr

@@ -13,10 +13,6 @@ impl<let KEY_LIMBS: u32> RSAPubkey<KEY_LIMBS> {
     pub fn new(modulus: [Field; KEY_LIMBS], redc: [Field; KEY_LIMBS]) -> Self {
         Self { modulus, redc }
     }
-
-    pub fn hash(self) -> Field {
-        pedersen_hash(self.modulus)
-    }
 }
 
 impl RSAPubkey<KEY_LIMBS_1024> {
@@ -36,6 +32,22 @@ impl RSAPubkey<KEY_LIMBS_1024> {
         // verify the DKIM signature over the header
         assert(verify_sha256_pkcs1v15(header_hash, signature, RSA_EXPONENT));
     }
+
+    pub fn hash(self) -> Field {
+        let mut dkim_preimage = [0; 9];
+        // compose first 4 limbs of modulus and redc
+        for i in 0..4 {
+            let modulus_hi = self.modulus[i * 2] * 2.pow_32(120);
+            let redc_hi = self.redc[i * 2] * 2.pow_32(120);
+            dkim_preimage[i] = modulus_hi + self.modulus[i * 2 + 1];
+            dkim_preimage[i + 4] = redc_hi + self.redc[i * 2 + 1];
+        }
+        // compose last two elements of redc and modulus together
+        let modulus_hi = self.modulus[8] * 2.pow_32(120);
+        dkim_preimage[8] = modulus_hi + self.redc[8];
+        // hash the pubkey
+        pedersen_hash(dkim_preimage)
+    }
 }
 
 impl RSAPubkey<KEY_LIMBS_2048> {
@@ -55,4 +67,17 @@ impl RSAPubkey<KEY_LIMBS_2048> {
         // verify the DKIM signature over the header
         assert(verify_sha256_pkcs1v15(header_hash, signature, RSA_EXPONENT));
     }
+
+    pub fn hash(self) -> Field {
+        let mut dkim_preimage = [0; 18];
+        // compose limbs
+        for i in 0..9 {
+            let modulus_hi = self.modulus[i * 2] * 2.pow_32(120);
+            let redc_hi = self.redc[i * 2] * 2.pow_32(120);
+            dkim_preimage[i] = modulus_hi + self.modulus[i * 2 + 1];
+            dkim_preimage[i + 9] = redc_hi + self.redc[i * 2 + 1];
+        }
+        // hash the pubkey
+        pedersen_hash(dkim_preimage)
+    }
 }

lib/src/headers/body_hash.nr

@@ -1,8 +1,8 @@
-use base64::BASE64_DECODER;
 use crate::{
-    Sequence, BODY_HASH_BASE64_LENGTH, MAX_DKIM_HEADER_FIELD_LENGTH,
-    headers::constrain_header_field,
+    BODY_HASH_BASE64_LENGTH, headers::constrain_header_field, MAX_DKIM_HEADER_FIELD_LENGTH,
+    Sequence,
 };
+use base64::BASE64_DECODER;
 
 /**
  * Constrained access to the body hash in the header
@@ -27,16 +27,19 @@ pub fn get_body_hash<let MAX_HEADER_LENGTH: u32>(
     // constrain access to the body hash
     assert(
         body_hash_index > dkim_header_field_sequence.index
-            & body_hash_index < dkim_header_field_sequence.end_index(),
+            & body_hash_index < dkim_header_field_sequence.end_index() + 1,
         "Body hash index accessed outside of DKIM header field",
     );
-    let bh_prefix: [u8; 3] = comptime { "bh=".as_bytes() };
-    for i in 0..3 {
-        assert(
-            header.get_unchecked(body_hash_index - 3 + i) == bh_prefix[i],
-            "No 'bh=' prefix found at asserted bh index",
-        );
+    let bh_prefix: [u8; 5] = comptime { "; bh=".as_bytes() };
+    for i in 0..5 {
+        let character = header.get_unchecked(body_hash_index - 5 + i);
+        assert(character == bh_prefix[i], "No 'bh=' prefix found at asserted bh index");
     }
+    let bh_suffix: u8 = comptime { ";".as_bytes()[0] };
+    assert(
+        header.get_unchecked(body_hash_index + BODY_HASH_BASE64_LENGTH) == bh_suffix,
+        "No ';' suffix found at asserted bh index",
+    );
     // get the body hash
     get_body_hash_unsafe(header, body_hash_index)
 }

lib/src/headers/mod.nr

@@ -56,7 +56,7 @@ pub fn constrain_header_field<let MAX_HEADER_LENGTH: u32, let MAX_HEADER_FIELD_L
     );
     // check the header field is uninterrupted
     let start_index = header_field_sequence.index + HEADER_FIELD_NAME_LENGTH + 1;
-    for i in (HEADER_FIELD_NAME_LENGTH + 1)..MAX_HEADER_FIELD_LENGTH {
+    for i in 0..MAX_HEADER_FIELD_LENGTH {
         // is it safe enough to cut this constraint cost in half by not checking lf? i think so
         let index = start_index + i;
         if (index < header_field_sequence.index + header_field_sequence.length) {

lib/src/lib.nr

@@ -39,28 +39,6 @@ global EMAIL_ADDRESS_CHAR_TABLE: [u8; 123] = [
     0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
 ];
 
-/**
- * Standard outputs that essentially every email circuit will need to export (alongside app-specific outputs)
- * @notice if you only need the pubkey hash just import pedersen and hash away 
- *
- * @param pubkey - the BN limbs of the DKIM RSA pubkey
- * @param signature - the BN limbs of the DKIM RSA signature
- * @returns
- *        0: Pedersen hash of DKIM public key (root of trust)
- *        1: Pedersen hash of DKIM signature (email nullifier)
- */
-pub fn standard_outputs<let KEY_BYTE_LENGTH: u32>(
-    pubkey: [Field; KEY_BYTE_LENGTH],
-    signature: [Field; KEY_BYTE_LENGTH],
-) -> [Field; 2] {
-    // create pedersen hash of DKIM signing key to minimize public outputs
-    let pubkey_hash = pedersen_hash(pubkey);
-    // create email nullifier for email
-    let email_nullifier = pedersen_hash(signature);
-    // output the root of trust and email nullifier
-    [pubkey_hash, email_nullifier]
-}
-
 /**
  * Default email verification function
  * @dev use #[zkemail] attribute macro to apply other functionality

lib/src/tests/mod.nr

@@ -167,6 +167,14 @@ mod header_field_access {
         let _ = get_body_hash(dkim_field, malicious_sequence, malicious_body_hash_index);
     }
 
+    #[test(should_fail_with = "No 'bh=' prefix found at asserted bh index")]
+    fn test_malicious_body_hash_index() {
+        // tests against "dkim-signature: v=1; a=rsa-sha256; d=example.com; s=selector; c=relaxed/relaxed; q=dns/txt; t=1683849600; x=1684454400; h=from:to:subject:date; z=From:[email protected]|To:[email protected]|Subject:Hello|Date:Thu, 11 May 2023 15:00:00 -0700; bh=2jUSOH9NhtVGCaWpZT2ncBgaamXkef9OgICHkqfsmKY=; b="
+        let (header, body_hash_index) = EmailLarge::tampered_dkim_field();
+        let sequence: Sequence = Sequence { index: 0, length: header.len() };
+        let _ = get_body_hash(header, sequence, body_hash_index);
+    }
+
     #[test(should_fail_with = "Header field must end with CRLF")]
     fn test_header_field_sequence_overflow_end() {
         // make sequence extend beyond the end of the header field

lib/src/tests/test_inputs.nr

@@ -178,6 +178,31 @@ pub(crate) mod EmailLarge {
         let dkim_sequence_start = DKIM_HEADER_SEQUENCE.index;
         dkim_sequence_start - 40
     }
+
+    pub unconstrained fn tampered_dkim_field() -> (BoundedVec<u8, 338>, u32) {
+        let header: BoundedVec<u8, 338> = BoundedVec::from_array([
+            100, 107, 105, 109, 45, 115, 105, 103, 110, 97, 116, 117, 114, 101, 58, 32, 118, 61, 49,
+            59, 32, 97, 61, 114, 115, 97, 45, 115, 104, 97, 50, 53, 54, 59, 32, 100, 61, 101, 120,
+            97, 109, 112, 108, 101, 46, 99, 111, 109, 59, 32, 115, 61, 115, 101, 108, 101, 99, 116,
+            111, 114, 59, 32, 99, 61, 114, 101, 108, 97, 120, 101, 100, 47, 114, 101, 108, 97, 120,
+            101, 100, 59, 32, 113, 61, 100, 110, 115, 47, 116, 120, 116, 59, 32, 116, 61, 49, 54,
+            56, 51, 56, 52, 57, 54, 48, 48, 59, 32, 120, 61, 49, 54, 56, 52, 52, 53, 52, 52, 48, 48,
+            59, 32, 104, 61, 102, 114, 111, 109, 58, 116, 111, 58, 115, 117, 98, 106, 101, 99, 116,
+            58, 100, 97, 116, 101, 59, 32, 122, 61, 70, 114, 111, 109, 58, 98, 104, 61, 55, 120, 81,
+            77, 68, 117, 111, 86, 86, 85, 52, 109, 48, 87, 48, 87, 82, 86, 83, 114, 86, 88, 77, 101,
+            71, 83, 73, 65, 83, 115, 110, 117, 99, 75, 57, 100, 74, 115, 114, 99, 43, 118, 85, 61,
+            64, 100, 111, 109, 97, 105, 110, 46, 99, 111, 109, 124, 84, 111, 58, 114, 101, 99, 105,
+            112, 105, 101, 110, 116, 64, 101, 120, 97, 109, 112, 108, 101, 46, 110, 101, 116, 124,
+            83, 117, 98, 106, 101, 99, 116, 58, 72, 101, 108, 108, 111, 124, 68, 97, 116, 101, 58,
+            84, 104, 117, 44, 32, 49, 49, 32, 77, 97, 121, 32, 50, 48, 50, 51, 32, 49, 53, 58, 48,
+            48, 58, 48, 48, 32, 45, 48, 55, 48, 48, 59, 32, 98, 104, 61, 50, 106, 85, 83, 79, 72,
+            57, 78, 104, 116, 86, 71, 67, 97, 87, 112, 90, 84, 50, 110, 99, 66, 103, 97, 97, 109,
+            88, 107, 101, 102, 57, 79, 103, 73, 67, 72, 107, 113, 102, 115, 109, 75, 89, 61, 59, 32,
+            98, 61,
+        ]);
+        let body_hash_index: u32 = 151;
+        (header, body_hash_index)
+    }
 }
 
 pub(crate) mod EmailAddresses {