CVE-2019-1010263: Improper Verification of Cryptographic Signature (CWE-347)
value: Perl Crypt::JWT prior to 0.023 is affected by: Incorrect Access Control. The impact is: allow attackers to bypass authentication by providing a token by crafting with hmac(). The component is: JWT.pm, line 614. The attack vector is: network connectivity. The fixed version is: after commit b98a59b42ded9f9e51b2560410106207c2152d6c.
CWE description: The product does not verify, or incorrectly verifies, the cryptographic signature for data.
CVSS v3 base score: 9.8
Published: 2019-07-17T21:15Z
Repository: https://github.com/DCIT/perl-Crypt-JWT
Language: Other
Files changed: README, lib/Crypt/JWT.pm, t/compile.t, t/jws_no_key.t
Commit date: 2018-09-01 18:04:14 +0200
Commit message: fix JWS signature validation (proper jwk header + kid_keys handling)

Diff: diff --git a/README b/README
index 20d018f..c1f84c2 100644
--- a/README
+++ b/README
@@ -215,14 +215,31 @@ FUNCTIONS
 
           use LWP::Simple;
           my $google_certs = get('https://www.googleapis.com/oauth2/v1/certs');
           my $payload = decode_jwt(token => $t, kid_keys => $google_certs);
 
-        When the token header contains 'kid' item the corresponding key is
+        When the token header contains "kid" item the corresponding key is
         looked up in "kid_keys" list and used for token decoding (you do not
         need to pass the explicit key via "key" parameter).
 
+        CHANGED in 0.23: When "kid_keys" is specified it croaks if token
+        header does not contain "kid" value or if "kid" was not found in
+        "kid_keys".
+
+    key_from_jwk_header
+        ADDED in 0.23
+
+        1 - use "jwk" header value for validating JWS signature if neither
+        "key" nor "kid_keys" specified, BEWARE: DANGEROUS, UNSECURE!!!
+
+        0 (default) - ignore "jwk" header value when validating JWS
+        signature
+
+        Keep in mind that enabling "key_from_jwk_header" requires "jwk"
+        header to exist and be an valid RSA/ECDSA public key (otherwise it
+        croaks).
+
     allow_none
         1 - accept JWS tokens with "none" 'alg' header value (which means
         that token has no signature), BEWARE: DANGEROUS, UNSECURE!!!
 
         0 (default) - do not allow JWS with "none" 'alg' header value
diff --git a/lib/Crypt/JWT.pm b/lib/Crypt/JWT.pm
index 33c1d26..20b384a 100644
--- a/lib/Crypt/JWT.pm
+++ b/lib/Crypt/JWT.pm
@@ -74,12 +74,13 @@ sub _prepare_oct_key {
   croak "JWT: invalid oct key";
 }
 
 sub _kid_lookup {
   my ($kid, $kid_keys, $alg) = @_;
-  $kid_keys = eval { decode_json($kid_keys) } unless ref $kid_keys;
-  return undef unless ref $kid_keys eq 'HASH';
+  return undef if !defined $kid || !defined $alg;
+  $kid_keys = eval { decode_json($kid_keys) } if $kid_keys && !ref $kid_keys;
+  croak "JWT: kid_keys must be a HASHREF or a valid JSON/HASH" if ref $kid_keys ne 'HASH';
   my $found;
   if (exists $kid_keys->{keys} && ref $kid_keys->{keys} eq 'ARRAY') {
     #FORMAT: { keys => [ {kid=>'A', kty=>?, ...}, {kid=>'B', kty=>?, ...} ] }
     for (@{$kid_keys->{keys}}) {
       if ($_->{kid} && $_->{kty} && $_->{kid} eq $kid) {
@@ -428,11 +429,10 @@ sub _encode_jwe {
   # compress payload
   $payload = _payload_zip($payload, $header, $args{zip}) if $args{zip}; # may set some header items
   # prepare header
   $header->{alg} = $alg;
   $header->{enc} = $enc;
-  #REMOVED: $header->{typ} = 'JWT' if !exists $header->{typ} && $args{auto_typ};
   # key
   croak "JWE: missing 'key'" if !$args{key};
   my $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
   # prepare cek
   my ($cek, $ecek) = _encrypt_jwe_cek($key, $header); # adds some header items
@@ -462,15 +462,22 @@ sub _decode_jwe {
   croak "JWE: invalid ecek part"   if $b64u_ecek   && !$ecek;
   croak "JWE: invalid ct part"     if $b64u_ct     && !$ct;
   croak "JWE: invalid iv part"     if $b64u_iv     && !$iv;
   croak "JWE: invalid tag part"    if $b64u_tag    && !$tag;
 
-  my $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
-  if ($header->{kid} && $args{kid_keys}) {
+  my $key;
+  if (exists $args{key}) {
+    $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
+  }
+  elsif (exists $args{kid_keys}) {
+    # BEWARE: stricter approach since 0.23
+    # when 'kid_keys' specified it croaks if header doesn't contain 'kid' value or if 'kid' wasn't found in 'kid_keys'
     my $k = _kid_lookup($header->{kid}, $args{kid_keys}, $header->{alg});
-    $key = $k if defined $k;
+    croak "JWE: kid_keys lookup failed" if !defined $k;
+    $key = $k;
   }
+  croak "JWE: missing key" if !defined $key;
 
   my $aa = $args{accepted_alg};
   if (ref($aa) eq 'Regexp') {
     croak "JWE: alg '$header->{alg}' does not match accepted_alg" if $header->{alg} !~ $aa;
   }
@@ -486,11 +493,10 @@ sub _decode_jwe {
   elsif ($ae && (ref($ae) eq 'ARRAY' || !ref($ae))) {
     my %acce = ref $ae ? map { $_ => 1 } @$ae : ( $ae => 1 );
     croak "JWE: enc '$header->{enc}' not in accepted_enc" if !$acce{$header->{enc}};
   }
 
-  croak "JWE: missing 'key'" if !$key;
   $header = { %$shared_unprotected, %$unprotected, %$header }; # merge headers
   my $cek = _decrypt_jwe_cek($ecek, $key, $header);
   my $aad = defined $b64u_aad ? "$b64u_header.$b64u_aad" : $b64u_header;
   my $payload = _decrypt_jwe_payload($cek, $header->{enc}, $aad, $ct, $iv, $tag);
   $payload = _payload_unzip($payload, $header->{zip}) if $header->{zip};
@@ -571,11 +577,10 @@ sub _encode_jws {
   $payload = _payload_zip($payload, $header, $args{zip}) if $args{zip}; # may set some header items
   # encode payload
   my $b64u_payload = encode_b64u($payload);
   # prepare header
   $header->{alg} = $alg;
-  #REMOVED: $header->{typ} = 'JWT' if !exists $header->{typ} && $args{auto_typ};
   # encode header
   my $json_header = encode_json($header);
   my $b64u_header = encode_b64u($json_header);
   # key
   croak "JWS: missing 'key'" if !$args{key} && $alg ne 'none';
@@ -590,33 +595,55 @@ sub _decode_jws {
   my $header = _b64u_to_hash($b64u_header);
   croak "JWS: invalid header part" if $b64u_header && !$header;
   $unprotected_header = {} if ref $unprotected_header ne 'HASH';
 
   if (!$args{ignore_signature}) {
-    my $aa = $args{accepted_alg};
-    if (ref($aa) eq 'Regexp') {
-      croak "JWS: alg '$header->{alg}' does not match accepted_alg" if $header->{alg} !~ $aa;
-    }
-    elsif ($aa && (ref($aa) eq 'ARRAY' || !ref($aa))) {
-      my %acca = ref $aa ? map { $_ => 1 } @$aa : ( $aa => 1 );
-      croak "JWS: alg '$header->{alg}' not in accepted_alg" if !$acca{$header->{alg}};
-    }
     my $alg = $header->{alg};
     croak "JWS: missing header 'alg'" unless $alg;
     croak "JWS: alg 'none' not allowed" if $alg eq 'none' && !$args{allow_none};
-    # key
-    my $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
-    my $kid = exists $header->{kid} ? $header->{kid} : $unprotected_header->{kid};
-    if (!defined $key && defined $kid && $args{kid_keys}) {
-      my $k = _kid_lookup($kid, $args{kid_keys}, $alg);
-      $key = $k if defined $k;
+    croak "JWS: alg 'none' expects no signature" if $alg eq 'none' && defined $b64u_sig && length($b64u_sig) > 0;
+
+    my $aa = $args{accepted_alg};
+    if (ref $aa eq 'Regexp') {
+      croak "JWS: alg '$alg' does not match accepted_alg" if $alg !~ $aa;
+    }
+    elsif (ref $aa eq 'ARRAY') {
+      my %acca = map { $_ => 1 } @$aa;
+      croak "JWS: alg '$alg' not in accepted_alg" if !$acca{$alg};
+    }
+    elsif (defined $aa) {
+      croak "JWS: alg '$alg' not accepted_alg" if $aa ne $alg;
+    }
+
+    if ($alg ne 'none') {
+      my $key;
+      if (exists $args{key}) {
+        $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
+      }
+      elsif (exists $args{kid_keys}) {
+        # BEWARE: stricter approach since 0.23
+        # when 'kid_keys' specified it croaks if header doesn't contain 'kid' value or if 'kid' wasn't found in 'kid_keys'
+        my $kid = exists $header->{kid} ? $header->{kid} : $unprotected_header->{kid};
+        my $k = _kid_lookup($kid, $args{kid_keys}, $alg);
+        croak "JWS: kid_keys lookup failed" if !defined $k;
+        $key = $k;
+      }
+      elsif ($args{key_from_jwk_header}) {
+        # BEWARE: stricter approach since 0.23
+        # - header 'jwk' is by default ignored (unless given: key_from_jwk_header => 1)
+        # - only RSA/ECDSA public keys are accepted
+        my $k = $header->{jwk};
+        croak "JWS: jwk header does not contain a key" if !defined $k || ref($k) ne 'HASH' || !defined $k->{kty};
+        croak "JWS: jwk header allowed only for RSA/ECDSA" if $alg !~ /^(RS|PS|ES)/ || $k->{kty} !~ /^(RSA|EC)$/;
+        croak "JWS: jwk header must be a public key" if $k->{d} || $k->{p} || $k->{q} || $k->{dp} || $k->{dq} || $k->{qi};
+        $key = $k;
+      }
+      croak "JWS: missing key" if !defined $key;
+
+      my $valid = _verify_jws($b64u_header, $b64u_payload, $b64u_sig, $alg, $key);
+      croak "JWS: invalid signature" if !$valid;
     }
-    # if no key given, try to use 'jwk' value from header
-    $key = $header->{jwk} if !$key && $header->{jwk};
-    croak "JWS: missing 'key'" if !$key && $alg ne 'none';
-    my $valid = _verify_jws($b64u_header, $b64u_payload, $b64u_sig, $alg, $key);
-    croak "JWS: decode failed" if !$valid;
   }
   my $payload = decode_b64u($b64u_payload);
   croak "JWS: invalid payload part" if $b64u_payload && !$payload;
   $payload = _payload_unzip($payload, $header->{zip}) if $header->{zip};
   $payload = _payload_dec($payload, $args{decode_payload});
@@ -947,13 +974,26 @@ Since 0.19 we also support:
 
   use LWP::Simple;
   my $google_certs = get('https://www.googleapis.com/oauth2/v1/certs');
   my $payload = decode_jwt(token => $t, kid_keys => $google_certs);
 
-When the token header contains 'kid' item the corresponding key is looked up in C<kid_keys> list and used for token
+When the token header contains C<kid> item the corresponding key is looked up in C<kid_keys> list and used for token
 decoding (you do not need to pass the explicit key via C<key> parameter).
 
+B<CHANGED in 0.23:> When C<kid_keys> is specified it croaks if token header does not contain C<kid> value or
+if C<kid> was not found in C<kid_keys>.
+
+=item key_from_jwk_header
+
+B<ADDED in 0.23>
+
+C<1> - use C<jwk> header value for validating JWS signature if neither C<key> nor C<kid_keys> specified, B<BEWARE: DANGEROUS, UNSECURE!!!>
+
+C<0> (default) - ignore C<jwk> header value when validating JWS signature
+
+Keep in mind that enabling C<key_from_jwk_header> requires C<jwk> header to exist and be an valid RSA/ECDSA public key (otherwise it croaks).
+
 =item allow_none
 
 C<1> - accept JWS tokens with C<none> 'alg' header value (which means that token has no signature), B<BEWARE: DANGEROUS, UNSECURE!!!>
 
 C<0> (default) - do not allow JWS with C<none> 'alg' header value
diff --git a/t/compile.t b/t/compile.t
index c321383..9c8f1b6 100644
--- a/t/compile.t
+++ b/t/compile.t
@@ -1,7 +1,9 @@
 use strict;
 use warnings;
 
-use Test::More tests => 2;
+use Test::More tests => 3;
 
 use_ok('Crypt::KeyWrap');
 use_ok('Crypt::JWT');
+
+is($Crypt::KeyWrap::VERSION, $Crypt::JWT::VERSION, 'consistent version');
diff --git a/t/jws_no_key.t b/t/jws_no_key.t
index fab1382..4ee2ecb 100644
--- a/t/jws_no_key.t
+++ b/t/jws_no_key.t
@@ -7,11 +7,11 @@ plan tests => 2;
 use Crypt::JWT qw(encode_jwt decode_jwt);
 
 my $jws =
   'eyJqd2siOnsiZSI6IkFRQUIiLCJuIjoieVFJdnpEU3h2a2EzQTNhVFYzS2Yza29PeElWMjNqZGlaa1BkOU8xb3RsN0JYLWZJS2dEYk00QnBHSkxZLUhrTG5aZUxpcXgwSFpKaF94U09IVXhWNnVpLUpIU00yZkFrTnEzMHd4QzMycDZmVDk2b3RuT3ZsTEhPTVVpNEZwUFR0NDVFQmcyemlqRXRfRWNFM3g0OFJjT2ZQVGk3SDBmWnhBdXVYcmJrYmU1SHFqczVxVWp2bDFKWUdKdTA1TlItdnE2NUwyUC1oOFA5eUJBT1pRZjhRMVhBSGg1RlFQd08tQjZ3T1p6aTNjeTEtRUhXZkhpWXpxeTMxWU01ZmxIaFZ4QndWRmUyMUlINEh3WWp2SE5KMURFaEl2R2FSQTBWc09ZNlFqVUxPS19XTVlQVnExc211TmdEZThlZ1V4RnV2R2N4aWJ4NTUydHJkSHVBaWFUVGlRIiwia3R5IjoiUlNBIn0sImFsZyI6IlJTMjU2Iiwibm9uY2UiOm51bGx9.eyJjb250YWN0IjpbIm1haWx0bzpmQGcudGxkIl0sInJlc291cmNlIjoibmV3LXJlZyJ9.wrY6y0kvA3qgR38ZuAA471ygN9fmSHdfWDIayjkBKGmeGbn0f30_LQBC9FiFDFgFJ8Owyy3bOkPWvHx7yhRnP5XnEYdzNtKy4t2LyKq_JSEVQf6p1zycsVaxVLCmZ6ZbRidxIFLhbkcmu2uc4BEVGQQEj3UesccIv-AS2JCQFqK5ca-HQeaLEMntXOz5p7DYZtauYjHuXQ60i25mClm51jScJfP-wk7yYnnohGYKDinwiYlH4Nw8p4yElzWL1dI-U8fiFoxnduGaflPIZ80hkk0p7delrZt3RvmaDdu4cLJ16TgrMw_nMZfbAK0IJXByAsbej78HwIAchdzHyRPmgA';
 
-my ( $header, $payload ) = decode_jwt( token => $jws, decode_header => 1 );
+my ( $header, $payload ) = decode_jwt( token => $jws, decode_header => 1, key_from_jwk_header => 1 );
 
 is_deeply(
     $header,
     {
         'alg' => 'RS256',

Vulnerable code:         When the token header contains 'kid' item the corresponding key is
  $kid_keys = eval { decode_json($kid_keys) } unless ref $kid_keys;
  return undef unless ref $kid_keys eq 'HASH';
  #REMOVED: $header->{typ} = 'JWT' if !exists $header->{typ} && $args{auto_typ};
  my $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
  if ($header->{kid} && $args{kid_keys}) {
    $key = $k if defined $k;
  croak "JWE: missing 'key'" if !$key;
  #REMOVED: $header->{typ} = 'JWT' if !exists $header->{typ} && $args{auto_typ};
    my $aa = $args{accepted_alg};
    if (ref($aa) eq 'Regexp') {
      croak "JWS: alg '$header->{alg}' does not match accepted_alg" if $header->{alg} !~ $aa;
    }
    elsif ($aa && (ref($aa) eq 'ARRAY' || !ref($aa))) {
      my %acca = ref $aa ? map { $_ => 1 } @$aa : ( $aa => 1 );
      croak "JWS: alg '$header->{alg}' not in accepted_alg" if !$acca{$header->{alg}};
    }
    # key
    my $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
    my $kid = exists $header->{kid} ? $header->{kid} : $unprotected_header->{kid};
    if (!defined $key && defined $kid && $args{kid_keys}) {
      my $k = _kid_lookup($kid, $args{kid_keys}, $alg);
      $key = $k if defined $k;
    # if no key given, try to use 'jwk' value from header
    $key = $header->{jwk} if !$key && $header->{jwk};
    croak "JWS: missing 'key'" if !$key && $alg ne 'none';
    my $valid = _verify_jws($b64u_header, $b64u_payload, $b64u_sig, $alg, $key);
    croak "JWS: decode failed" if !$valid;
When the token header contains 'kid' item the corresponding key is looked up in C<kid_keys> list and used for token
use Test::More tests => 2;
my ( $header, $payload ) = decode_jwt( token => $jws, decode_header => 1 );
Fixed code:         When the token header contains "kid" item the corresponding key is
        CHANGED in 0.23: When "kid_keys" is specified it croaks if token
        header does not contain "kid" value or if "kid" was not found in
        "kid_keys".

    key_from_jwk_header
        ADDED in 0.23

        1 - use "jwk" header value for validating JWS signature if neither
        "key" nor "kid_keys" specified, BEWARE: DANGEROUS, UNSECURE!!!

        0 (default) - ignore "jwk" header value when validating JWS
        signature

        Keep in mind that enabling "key_from_jwk_header" requires "jwk"
        header to exist and be an valid RSA/ECDSA public key (otherwise it
        croaks).

  return undef if !defined $kid || !defined $alg;
  $kid_keys = eval { decode_json($kid_keys) } if $kid_keys && !ref $kid_keys;
  croak "JWT: kid_keys must be a HASHREF or a valid JSON/HASH" if ref $kid_keys ne 'HASH';
  my $key;
  if (exists $args{key}) {
    $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
  }
  elsif (exists $args{kid_keys}) {
    # BEWARE: stricter approach since 0.23
    # when 'kid_keys' specified it croaks if header doesn't contain 'kid' value or if 'kid' wasn't found in 'kid_keys'
    croak "JWE: kid_keys lookup failed" if !defined $k;
    $key = $k;
  croak "JWE: missing key" if !defined $key;
    croak "JWS: alg 'none' expects no signature" if $alg eq 'none' && defined $b64u_sig && length($b64u_sig) > 0;

    my $aa = $args{accepted_alg};
    if (ref $aa eq 'Regexp') {
      croak "JWS: alg '$alg' does not match accepted_alg" if $alg !~ $aa;
    }
    elsif (ref $aa eq 'ARRAY') {
      my %acca = map { $_ => 1 } @$aa;
      croak "JWS: alg '$alg' not in accepted_alg" if !$acca{$alg};
    }
    elsif (defined $aa) {
      croak "JWS: alg '$alg' not accepted_alg" if $aa ne $alg;
    }

    if ($alg ne 'none') {
      my $key;
      if (exists $args{key}) {
        $key = defined $args{keypass} ? [$args{key}, $args{keypass}] : $args{key};
      }
      elsif (exists $args{kid_keys}) {
        # BEWARE: stricter approach since 0.23
        # when 'kid_keys' specified it croaks if header doesn't contain 'kid' value or if 'kid' wasn't found in 'kid_keys'
        my $kid = exists $header->{kid} ? $header->{kid} : $unprotected_header->{kid};
        my $k = _kid_lookup($kid, $args{kid_keys}, $alg);
        croak "JWS: kid_keys lookup failed" if !defined $k;
        $key = $k;
      }
      elsif ($args{key_from_jwk_header}) {
        # BEWARE: stricter approach since 0.23
        # - header 'jwk' is by default ignored (unless given: key_from_jwk_header => 1)
        # - only RSA/ECDSA public keys are accepted
        my $k = $header->{jwk};
        croak "JWS: jwk header does not contain a key" if !defined $k || ref($k) ne 'HASH' || !defined $k->{kty};
        croak "JWS: jwk header allowed only for RSA/ECDSA" if $alg !~ /^(RS|PS|ES)/ || $k->{kty} !~ /^(RSA|EC)$/;
        croak "JWS: jwk header must be a public key" if $k->{d} || $k->{p} || $k->{q} || $k->{dp} || $k->{dq} || $k->{qi};
        $key = $k;
      }
      croak "JWS: missing key" if !defined $key;

      my $valid = _verify_jws($b64u_header, $b64u_payload, $b64u_sig, $alg, $key);
      croak "JWS: invalid signature" if !$valid;
When the token header contains C<kid> item the corresponding key is looked up in C<kid_keys> list and used for token
B<CHANGED in 0.23:> When C<kid_keys> is specified it croaks if token header does not contain C<kid> value or
if C<kid> was not found in C<kid_keys>.

=item key_from_jwk_header

B<ADDED in 0.23>

C<1> - use C<jwk> header value for validating JWS signature if neither C<key> nor C<kid_keys> specified, B<BEWARE: DANGEROUS, UNSECURE!!!>

C<0> (default) - ignore C<jwk> header value when validating JWS signature

Keep in mind that enabling C<key_from_jwk_header> requires C<jwk> header to exist and be an valid RSA/ECDSA public key (otherwise it croaks).

use Test::More tests => 3;

is($Crypt::KeyWrap::VERSION, $Crypt::JWT::VERSION, 'consistent version');
my ( $header, $payload ) = decode_jwt( token => $jws, decode_header => 1, key_from_jwk_header => 1 );