Merge pull request from GHSA-7f9x-gw85-8grf

Author: lestrrat

Changes

@@ -4,6 +4,15 @@ Changes
 v2 has many incompatibilities with v1. To see the full list of differences between
 v1 and v2, please read the Changes-v2.md file (https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes-v2.md)
 
+v2.0.18 UNRELEASED
+[Security Fixes]
+  * [jwe] A large number in p2c parameter for PBKDF2 based encryptions could cause a DoS attack,
+    similar to https://nvd.nist.gov/vuln/detail/CVE-2022-36083.  All users who use JWE via this
+    package should upgrade. While the JOSE spec allows for encryption using JWE on JWTs, users of
+    the `jwt` package are not immediately susceptible unless they explicitly try to decrypt
+    JWTs -- by default the `jwt` package verifies signatures, but does not decrypt messages.
+    [GHSA-7f9x-gw85-8grf]
+
 v2.0.17 20 Nov 2023
 [Bug Fixes]
   * [jws] Previously, `jws.UnregisterSigner` did not remove the previous signer instance when

jwe/jwe.go

@@ -10,6 +10,7 @@ import (
 	"crypto/rsa"
 	"fmt"
 	"io"
+	"sync"
 
 	"github.com/lestrrat-go/blackmagic"
 	"github.com/lestrrat-go/jwx/v2/internal/base64"
@@ -24,6 +25,20 @@ import (
 	"github.com/lestrrat-go/jwx/v2/x25519"
 )
 
+var muSettings sync.RWMutex
+var maxPBES2Count = 10000
+
+func Settings(options ...GlobalOption) {
+	muSettings.Lock()
+	defer muSettings.Unlock()
+	for _, option := range options {
+		switch option.Ident() {
+		case identMaxPBES2Count{}:
+			maxPBES2Count = option.Value().(int)
+		}
+	}
+}
+
 const (
 	fmtInvalid = iota
 	fmtCompact
@@ -702,6 +717,12 @@ func (dctx *decryptCtx) decryptContent(ctx context.Context, alg jwa.KeyEncryptio
 		if !ok {
 			return nil, fmt.Errorf("unexpected type for 'p2c': %T", count)
 		}
+		muSettings.RLock()
+		maxCount := maxPBES2Count
+		muSettings.RUnlock()
+		if countFlt > float64(maxCount) {
+			return nil, fmt.Errorf("invalid 'p2c' value")
+		}
 		salt, err := base64.DecodeString(saltB64Str)
 		if err != nil {
 			return nil, fmt.Errorf(`failed to b64-decode 'salt': %w`, err)

jwe/jwe_test.go

@@ -911,3 +911,51 @@ func TestGH1001(t *testing.T) {
 	require.Equal(t, "Lorem Ipsum", string(decrypted), `decrypted message should match`)
 	require.NotNil(t, cek, `cek should not be nil`)
 }
+
+func TestGHSA_7f9x_gw85_8grf(t *testing.T) {
+	token := []byte("eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMjU2R0NNIiwicDJjIjoyMDAwMDAwMDAwLCJwMnMiOiJNNzczSnlmV2xlX2FsSXNrc0NOTU9BIn0=.S8B1kXdIR7BM6i_TaGsgqEOxU-1Sgdakp4mHq7UVhn-_REzOiGz2gg.gU_LfzhBXtQdwYjh.9QUIS-RWkLc.m9TudmzUoCzDhHsGGfzmCA")
+	key, err := jwk.FromRaw([]byte(`abcdefg`))
+	require.NoError(t, err, `jwk.FromRaw should succeed`)
+
+	{
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+
+		done := make(chan struct{})
+		go func(t *testing.T, done chan struct{}) {
+			_, err := jwe.Decrypt(token, jwe.WithKey(jwa.PBES2_HS256_A128KW, key))
+			require.Error(t, err, `jwe.Decrypt should fail`)
+			close(done)
+		}(t, done)
+
+		select {
+		case <-done:
+		case <-ctx.Done():
+			require.Fail(t, "jwe.Decrypt should not block")
+		}
+	}
+
+	// NOTE: HAS GLOBAL EFFECT
+	// Should allow for timeout to occur
+	jwe.Settings(jwe.WithMaxPBES2Count(100000000000000000))
+
+	// put it back to normal after the test
+	defer jwe.Settings(jwe.WithMaxPBES2Count(10000))
+	{
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+
+		done := make(chan struct{})
+		go func(t *testing.T, done chan struct{}) {
+			_, _ = jwe.Decrypt(token, jwe.WithKey(jwa.PBES2_HS256_A128KW, key))
+			close(done)
+		}(t, done)
+
+		select {
+		case <-done:
+			require.Fail(t, "jwe.Decrypt should block")
+		case <-ctx.Done():
+			// timeout occurred as it should
+		}
+	}
+}

jwe/options.yaml

@@ -1,6 +1,9 @@
 package_name: jwe
 output: jwe/options_gen.go
 interfaces:
+  - name: GlobalOption
+    comment: |
+      GlobalOption describes options that changes global settings for this package
   - name: CompactOption
     comment: |
       CompactOption describes options that can be passed to `jwe.Compact`
@@ -129,3 +132,10 @@ options:
       
       This option is currently considered EXPERIMENTAL, and is subject to
       future changes across minor/micro versions.
+  - ident: MaxPBES2Count
+    interface: GlobalOption
+    argument_type: int
+    comment: |
+      WithMaxPBES2Count specifies the maximum number of PBES2 iterations
+      to use when decrypting a message. If not specified, the default
+      value of 10,000 is used.