CVE-2024-21491: Insufficient Information (NVD-CWE-noinfo)
value: Versions of the package svix before 1.17.0 are vulnerable to Authentication Bypass due to an issue in the verify function where signatures of different lengths are incorrectly compared. An attacker can bypass signature verification by providing a shorter signature that matches the beginning of the actual signature.**Note:**The attacker would need to know a victim uses the Rust library for verification,no easy way to automatically check that; and uses webhooks by a service that uses Svix, and then figure out a way to craft a malicious payload that will actually include all of the correct identifiers needed to trick the receivers to cause actual issues.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-02-13T05:15Z
Repository: https://github.com/svix/svix-webhooks
Language: Rust
Files changed: go/webhook_test.go, javascript/src/webhook.test.ts, rust/src/webhooks.rs
Commit date: 2024-02-06 19:11:22 +0200
Commit message: Rust verification library: fix comparison of different length signatures (#1190)

There was a bug in the code which meant that the signatures are only
compared up to the length of the shorter signature, which means that an
attacker can just pass `v1,` as the signature and that will always pass
verification.

This change fixes it so that the length of the signature is also taken
into account when comparing, to make sure that it's always the same
length before comparing.

Manually verified all of the other libraries are correct, and added
tests to JavaScript and Go (even though they were also not affected).

Many thanks to Fredrik Meringdal (@fmeringdal) for the report.
Diff: diff --git a/go/webhook_test.go b/go/webhook_test.go
index 1c5f2c93..37f86a13 100644
--- a/go/webhook_test.go
+++ b/go/webhook_test.go
@@ -99,10 +99,18 @@ func TestWebhook(t *testing.T) {
 			modifyPayload: func(tp *testPayload) {
 				tp.header.Set("svix-signature", "v1,Ceo5qEr07ixe2NLpvHk3FH9bwy/WavXrAFQ/9tdO6mc=")
 			},
 			expectedErr: true,
 		},
+		{
+			name:        "partial signature is invalid",
+			testPayload: newTestPayload(time.Now()),
+			modifyPayload: func(tp *testPayload) {
+				tp.header.Set("svix-signature", "v1,")
+			},
+			expectedErr: true,
+		},
 		{
 			name:        "old timestamp fails",
 			testPayload: newTestPayload(time.Now().Add(tolerance * -1)),
 			expectedErr: true,
 		},
diff --git a/javascript/src/webhook.test.ts b/javascript/src/webhook.test.ts
index 1ef2da7e..58c5410c 100644
--- a/javascript/src/webhook.test.ts
+++ b/javascript/src/webhook.test.ts
@@ -101,10 +101,27 @@ test("invalid signature throws error", () => {
   expect(() => {
     wh.verify(testPayload.payload, testPayload.header);
   }).toThrowError(WebhookVerificationError);
 });
 
+test("partial signature throws error", () => {
+  const wh = new Webhook("MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw");
+
+  const testPayload = new TestPayload();
+  testPayload.header["svix-signature"] = testPayload.header["svix-signature"].slice(0, 8);
+
+  expect(() => {
+    wh.verify(testPayload.payload, testPayload.header);
+  }).toThrowError(WebhookVerificationError);
+
+  testPayload.header["svix-signature"] = "v1,";
+
+  expect(() => {
+    wh.verify(testPayload.payload, testPayload.header);
+  }).toThrowError(WebhookVerificationError);
+});
+
 test("valid signature is valid and returns valid json", () => {
   const wh = new Webhook("MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw");
 
   const testPayload = new TestPayload();
 
diff --git a/rust/src/webhooks.rs b/rust/src/webhooks.rs
index 964c17c6..acf8bfe3 100644
--- a/rust/src/webhooks.rs
+++ b/rust/src/webhooks.rs
@@ -84,14 +84,17 @@ impl Webhook {
         msg_signature
             .split(' ')
             .filter_map(|x| x.split_once(','))
             .filter(|x| x.0 == SIGNATURE_VERSION)
             .any(|x| {
-                x.1.bytes()
-                    .zip(expected_signature.bytes())
-                    .fold(0, |acc, (a, b)| acc | (a ^ b))
-                    == 0
+                (x.1.len() == expected_signature.len())
+                    && (x
+                        .1
+                        .bytes()
+                        .zip(expected_signature.bytes())
+                        .fold(0, |acc, (a, b)| acc | (a ^ b))
+                        == 0)
             })
             .then_some(())
             .ok_or(WebhookError::InvalidSignature)
     }
 
@@ -221,10 +224,47 @@ mod tests {
         ] {
             assert!(wh.verify(payload, &headers).is_err());
         }
     }
 
+    #[test]
+    fn test_verify_partial_signature() {
+        let secret = "whsec_C2FVsBQIhrscChlQIMV+b5sSYspob7oD".to_owned();
+        let msg_id = "msg_27UH4WbU6Z5A5EzD8u03UvzRbpk";
+        let payload = br#"{"email":"test@example.com","username":"test_user"}"#;
+        let wh = Webhook::new(&secret).unwrap();
+
+        let signature = wh
+            .sign(msg_id, OffsetDateTime::now_utc().unix_timestamp(), payload)
+            .unwrap();
+
+        // Just `v1,`
+        for mut headers in [
+            get_svix_headers(msg_id, &signature),
+            get_unbranded_headers(msg_id, &signature),
+        ] {
+            let partial = format!(
+                "{},",
+                signature.split(',').collect::<Vec<&str>>().first().unwrap()
+            );
+            headers.insert(SVIX_MSG_SIGNATURE_KEY, partial.parse().unwrap());
+            headers.insert(UNBRANDED_MSG_SIGNATURE_KEY, partial.parse().unwrap());
+            assert!(wh.verify(payload, &headers).is_err());
+        }
+
+        // Non-empty but still partial signature (first few bytes)
+        for mut headers in [
+            get_svix_headers(msg_id, &signature),
+            get_unbranded_headers(msg_id, &signature),
+        ] {
+            let partial = &signature[0..8];
+            headers.insert(SVIX_MSG_SIGNATURE_KEY, partial.parse().unwrap());
+            headers.insert(UNBRANDED_MSG_SIGNATURE_KEY, partial.parse().unwrap());
+            assert!(wh.verify(payload, &headers).is_err());
+        }
+    }
+
     #[test]
     fn test_verify_incorrect_timestamp() {
         let secret = "whsec_C2FVsBQIhrscChlQIMV+b5sSYspob7oD".to_owned();
         let msg_id = "msg_27UH4WbU6Z5A5EzD8u03UvzRbpk";
         let payload = br#"{"email":"test@example.com","username":"test_user"}"#;

Vulnerable code:                 x.1.bytes()
                    .zip(expected_signature.bytes())
                    .fold(0, |acc, (a, b)| acc | (a ^ b))
                    == 0
Fixed code: 		{
			name:        "partial signature is invalid",
			testPayload: newTestPayload(time.Now()),
			modifyPayload: func(tp *testPayload) {
				tp.header.Set("svix-signature", "v1,")
			},
			expectedErr: true,
		},
test("partial signature throws error", () => {
  const wh = new Webhook("MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw");

  const testPayload = new TestPayload();
  testPayload.header["svix-signature"] = testPayload.header["svix-signature"].slice(0, 8);

  expect(() => {
    wh.verify(testPayload.payload, testPayload.header);
  }).toThrowError(WebhookVerificationError);

  testPayload.header["svix-signature"] = "v1,";

  expect(() => {
    wh.verify(testPayload.payload, testPayload.header);
  }).toThrowError(WebhookVerificationError);
});

                (x.1.len() == expected_signature.len())
                    && (x
                        .1
                        .bytes()
                        .zip(expected_signature.bytes())
                        .fold(0, |acc, (a, b)| acc | (a ^ b))
                        == 0)
    #[test]
    fn test_verify_partial_signature() {
        let secret = "whsec_C2FVsBQIhrscChlQIMV+b5sSYspob7oD".to_owned();
        let msg_id = "msg_27UH4WbU6Z5A5EzD8u03UvzRbpk";
        let payload = br#"{"email":"test@example.com","username":"test_user"}"#;
        let wh = Webhook::new(&secret).unwrap();

        let signature = wh
            .sign(msg_id, OffsetDateTime::now_utc().unix_timestamp(), payload)
            .unwrap();

        // Just `v1,`
        for mut headers in [
            get_svix_headers(msg_id, &signature),
            get_unbranded_headers(msg_id, &signature),
        ] {
            let partial = format!(
                "{},",
                signature.split(',').collect::<Vec<&str>>().first().unwrap()
            );
            headers.insert(SVIX_MSG_SIGNATURE_KEY, partial.parse().unwrap());
            headers.insert(UNBRANDED_MSG_SIGNATURE_KEY, partial.parse().unwrap());
            assert!(wh.verify(payload, &headers).is_err());
        }

        // Non-empty but still partial signature (first few bytes)
        for mut headers in [
            get_svix_headers(msg_id, &signature),
            get_unbranded_headers(msg_id, &signature),
        ] {
            let partial = &signature[0..8];
            headers.insert(SVIX_MSG_SIGNATURE_KEY, partial.parse().unwrap());
            headers.insert(UNBRANDED_MSG_SIGNATURE_KEY, partial.parse().unwrap());
            assert!(wh.verify(payload, &headers).is_err());
        }
    }