CVE-2023-23928: Improper Verification of Cryptographic Signature (CWE-347)
value: reason-jose is a JOSE implementation in ReasonML and OCaml.`Jose.Jws.validate` does not check HS256 signatures. This allows tampering of JWS header and payload data if the service does not perform additional checks. Such tampering could expose applications using reason-jose to authorization bypass. Applications relying on JWS claims assertion to enforce security boundaries may be vulnerable to privilege escalation. This issue has been patched in version 0.8.2.
CWE description: The product does not verify, or incorrectly verifies, the cryptographic signature for data.
CVSS v3 base score: 9.8
Published: 2023-02-01T01:15Z
Repository: https://github.com/ulrikstrid/reason-jose
Language: Other
Files changed: jose/Jws.ml, test/RFC7520.ml
Commit date: 2023-01-31 16:46:55 +0100
Commit message: Merge pull request from GHSA-7jj9-6qwv-wpm7

* Add failing test

* Fix HS256 signature check

* Decode with Jwk.oct_to_sign_key helper

---------

Co-authored-by: Kai <nankeen@users.noreply.github.com>
Diff: diff --git a/jose/Jws.ml b/jose/Jws.ml
index c9656ff..9df1ef6 100644
--- a/jose/Jws.ml
+++ b/jose/Jws.ml
@@ -33,12 +33,22 @@ let verify_jwk (type a) ~(jwk : a Jwk.t) ~input_str str =
   | Jwk.Rsa_pub jwk -> (
       Mirage_crypto_pk.Rsa.PKCS1.sig_decode ~key:jwk.key str |> function
       | None -> Error `Invalid_signature
       | Some message -> Ok message)
   | Jwk.Oct jwk ->
-      Mirage_crypto.Hash.SHA256.hmac ~key:(Cstruct.of_string jwk.key) str
-      |> U_Result.return
+      Jwk.oct_to_sign_key jwk
+      |> U_Result.flat_map (fun key ->
+             let computed_signature =
+               Mirage_crypto.Hash.SHA256.hmac ~key (Cstruct.of_string input_str)
+             in
+             (* From RFC7518§3.2:
+              *   The comparison of the computed HMAC value to the JWS Signature
+              *   value MUST be done in a constant-time manner to thwart timing
+              *   attacks. *)
+             if Eqaf_cstruct.equal str computed_signature then
+               Ok computed_signature
+             else Error `Invalid_signature)
   | Jwk.Es256_pub pub_jwk ->
       let r, s = Cstruct.split str 32 in
       let message =
         Mirage_crypto.Hash.SHA256.digest (Cstruct.of_string input_str)
       in
@@ -76,31 +86,24 @@ let verify_internal (type a) ~(jwk : a Jwk.t) t =
   let payload_str = U_Base64.url_encode_string t.payload in
   let input_str = Printf.sprintf "%s.%s" t.raw_header payload_str in
   U_Base64.url_decode t.signature
   |> U_Result.map Cstruct.of_string
   |> U_Result.flat_map (verify_jwk ~jwk ~input_str)
-  |> U_Result.map (fun message ->
-         let token_hash =
-           input_str |> Cstruct.of_string |> Mirage_crypto.Hash.SHA256.digest
-         in
-         (* From RFC7518§3.2:
-          *   The comparison of the computed HMAC value to the JWS Signature
-          *   value MUST be done in a constant-time manner to thwart timing
-          *   attacks. *)
-         Eqaf_cstruct.equal message token_hash)
 
 let validate (type a) ~(jwk : a Jwk.t) t =
   let header = t.header in
   (match header.alg with
   | `RS256 -> Ok header.alg
   | `HS256 -> Ok header.alg
   | `ES256 -> Ok header.alg
   | `ES512 -> Ok header.alg
   | `Unsupported _ | `RSA_OAEP | `RSA1_5 | `None ->
       Error (`Msg "alg not supported for signing"))
-  |> U_Result.flat_map (fun _ -> verify_internal ~jwk t)
-  |> U_Result.map (fun _ -> t)
+  |> U_Result.flat_map (fun _alg ->
+         match verify_internal ~jwk t with
+         | Ok _sig -> Ok t
+         | Error e -> Error e)
 
 (* Assumes a well formed header. *)
 let sign ?header ~payload (jwk : Jwk.priv Jwk.t) =
   let header =
     match header with
diff --git a/test/RFC7520.ml b/test/RFC7520.ml
index 8916ad6..3ddbc1e 100644
--- a/test/RFC7520.ml
+++ b/test/RFC7520.ml
@@ -221,10 +221,21 @@ let jws_oct_tests =
             |> CCResult.flat_map (fun (jwk, header) ->
                    Jose.Jws.sign ~header ~payload:jws_payload jwk)
           in
           check_result_string "correct jws string" (Ok oct_jws)
             (CCResult.map Jose.Jws.to_string jws));
+      Alcotest.test_case "Cannot verify invalid signature" `Quick (fun () ->
+          let jwk =
+            Jose.Jwk.of_pub_json_string oct_sig_json |> CCResult.get_exn
+          in
+          let jws =
+            Jose.Jws.of_string
+              {|eyJhbGciOiJIUzI1NiIsImtpZCI6IjAxOGMwYWU1LTRkOWItNDcxYi1iZmQ2LWVlZjMxNGJjNzAzNyJ9.SXTigJlzIGEgZGFuZ2Vyb3VzIGJ1c2luZXNzLCBGcm9kbywgZ29pbmcgb3V0IHlvdXIgZG9vci4gWW91IHN0ZXAgb250byB0aGUgcm9hZCwgYW5kIGlmIHlvdSBkb24ndCBrZWVwIHlvdXIgZmVldCwgdGhlcmXigJlzIG5vIGtub3dpbmcgd2hlcmUgeW91IG1pZ2h0IGJlIHN3ZXB0IG9mZiB0by4.NO_GOOD_SIGNATURE|}
+          in
+          let validated_jws = CCResult.flat_map (Jose.Jws.validate ~jwk) jws in
+          check_result_string "validation fails" (Error `Invalid_signature)
+            (CCResult.map (fun jws -> Jose.Jws.(jws.signature)) validated_jws));
     ] )
 
 let rsa_priv_enc_json_5_1 =
   {|{"kty": "RSA",
 "kid": "frodo.baggins@hobbiton.example",

Vulnerable code:       Mirage_crypto.Hash.SHA256.hmac ~key:(Cstruct.of_string jwk.key) str
      |> U_Result.return
  |> U_Result.map (fun message ->
         let token_hash =
           input_str |> Cstruct.of_string |> Mirage_crypto.Hash.SHA256.digest
         in
         (* From RFC7518§3.2:
          *   The comparison of the computed HMAC value to the JWS Signature
          *   value MUST be done in a constant-time manner to thwart timing
          *   attacks. *)
         Eqaf_cstruct.equal message token_hash)
  |> U_Result.flat_map (fun _ -> verify_internal ~jwk t)
  |> U_Result.map (fun _ -> t)
Fixed code:       Jwk.oct_to_sign_key jwk
      |> U_Result.flat_map (fun key ->
             let computed_signature =
               Mirage_crypto.Hash.SHA256.hmac ~key (Cstruct.of_string input_str)
             in
             (* From RFC7518§3.2:
              *   The comparison of the computed HMAC value to the JWS Signature
              *   value MUST be done in a constant-time manner to thwart timing
              *   attacks. *)
             if Eqaf_cstruct.equal str computed_signature then
               Ok computed_signature
             else Error `Invalid_signature)
  |> U_Result.flat_map (fun _alg ->
         match verify_internal ~jwk t with
         | Ok _sig -> Ok t
         | Error e -> Error e)
      Alcotest.test_case "Cannot verify invalid signature" `Quick (fun () ->
          let jwk =
            Jose.Jwk.of_pub_json_string oct_sig_json |> CCResult.get_exn
          in
          let jws =
            Jose.Jws.of_string
              {|eyJhbGciOiJIUzI1NiIsImtpZCI6IjAxOGMwYWU1LTRkOWItNDcxYi1iZmQ2LWVlZjMxNGJjNzAzNyJ9.SXTigJlzIGEgZGFuZ2Vyb3VzIGJ1c2luZXNzLCBGcm9kbywgZ29pbmcgb3V0IHlvdXIgZG9vci4gWW91IHN0ZXAgb250byB0aGUgcm9hZCwgYW5kIGlmIHlvdSBkb24ndCBrZWVwIHlvdXIgZmVldCwgdGhlcmXigJlzIG5vIGtub3dpbmcgd2hlcmUgeW91IG1pZ2h0IGJlIHN3ZXB0IG9mZiB0by4.NO_GOOD_SIGNATURE|}
          in
          let validated_jws = CCResult.flat_map (Jose.Jws.validate ~jwk) jws in
          check_result_string "validation fails" (Error `Invalid_signature)
            (CCResult.map (fun jws -> Jose.Jws.(jws.signature)) validated_jws));