Merge pull request #23 from zkemail/audit/fix-bh-access

veredise audit fix: bh= may occur in other DKIM tags

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/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 {