From d01027d74c7376d66037a10f4f64af9af26a7e34 Mon Sep 17 00:00:00 2001 From: lestrrat <49281+lestrrat@users.noreply.github.com> Date: Thu, 7 Mar 2024 10:21:35 +0900 Subject: [PATCH] Merge pull request from GHSA-hj3v-m684-v259 * Add WithMaxDecompressBufferSize option for v1 * tweak test name * Tweak Changes * tweak documentation --- Changes | 7 ++++++ jwe/compress.go | 32 ++++++++++++++++++++++--- jwe/jwe.go | 21 ++++++++++++---- jwe/jwe_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ jwe/message.go | 3 ++- jwe/options.go | 9 +++++++ 6 files changed, 128 insertions(+), 8 deletions(-) diff --git a/Changes b/Changes index 897ab3c66..f3932b02e 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,13 @@ Changes ======= +v1.2.29 UNRELEASED + * [jwe] Added `jwe.Settings(jwe.WithMaxDecompressBufferSize(int64))` to specify the + maximum size of a decompressed JWE payload. The default value is 10MB. If you + are compressing payloads greater than this, you need to explicitly set it. + + Unlike in v2, there is no way to set this globally. Please use v2 if this is required. + v1.2.28 09 Jan 2024 [Security Fixes] * [jws] JWS messages formated in full JSON format (i.e. not the compact format, which diff --git a/jwe/compress.go b/jwe/compress.go index e3836a693..53dbd2b5a 100644 --- a/jwe/compress.go +++ b/jwe/compress.go @@ -3,15 +3,41 @@ package jwe import ( "bytes" "compress/flate" - "io/ioutil" + "io" "github.com/lestrrat-go/jwx/internal/pool" "github.com/lestrrat-go/jwx/jwa" "github.com/pkg/errors" ) -func uncompress(plaintext []byte) ([]byte, error) { - return ioutil.ReadAll(flate.NewReader(bytes.NewReader(plaintext))) +func uncompress(src []byte, maxBufferSize int64) ([]byte, error) { + var dst bytes.Buffer + r := flate.NewReader(bytes.NewReader(src)) + defer r.Close() + var buf [16384]byte + var sofar int64 + for { + n, readErr := r.Read(buf[:]) + sofar += int64(n) + if sofar > maxBufferSize { + return nil, errors.New(`compressed payload exceeds maximum allowed size`) + } + if readErr != nil { + // if we have a read error, and it's not EOF, then we need to stop + if readErr != io.EOF { + return nil, errors.Wrap(readErr, `failed to read inflated data`) + } + } + + if _, err := dst.Write(buf[:n]); err != nil { + return nil, errors.Wrap(err, `failed to write inflated data`) + } + + if readErr != nil { + // if it got here, then readErr == io.EOF, we're done + return dst.Bytes(), nil + } + } } func compress(plaintext []byte, alg jwa.CompressionAlgorithm) ([]byte, error) { diff --git a/jwe/jwe.go b/jwe/jwe.go index d26d14c22..ae0705ae5 100644 --- a/jwe/jwe.go +++ b/jwe/jwe.go @@ -7,6 +7,7 @@ import ( "bytes" "crypto/ecdsa" "crypto/rsa" + "fmt" "io" "io/ioutil" @@ -25,7 +26,7 @@ import ( var registry = json.NewRegistry() -// Encrypt takes the plaintext payload and encrypts it in JWE compact format. +// Encrypt takes the pllaintext payload and encrypts it in JWE compact format. // `key` should be a public key, and it may be a raw key (e.g. rsa.PublicKey) or a jwk.Key // // Encrypt currently does not support multi-recipient messages. @@ -179,9 +180,10 @@ type DecryptCtx interface { } type decryptCtx struct { - alg jwa.KeyEncryptionAlgorithm - key interface{} - msg *Message + alg jwa.KeyEncryptionAlgorithm + key interface{} + msg *Message + maxDecompressBufferSize int64 } func (ctx *decryptCtx) Algorithm() jwa.KeyEncryptionAlgorithm { @@ -213,6 +215,11 @@ func (ctx *decryptCtx) SetMessage(m *Message) { // The JWE message can be either compact or full JSON format. // // `key` must be a private key. It can be either in its raw format (e.g. *rsa.PrivateKey) or a jwk.Key +// +// The decrypted payload must be smaller than the amount specified by the +// `jwe.WithMaxDecompressBufferSize` setting, which defaults to 10MB. +// +// jwe.Decrypt(..., jwe.WithMaxDecompressBufferSize(250*1024)) func Decrypt(buf []byte, alg jwa.KeyEncryptionAlgorithm, key interface{}, options ...DecryptOption) ([]byte, error) { var ctx decryptCtx ctx.key = key @@ -220,6 +227,8 @@ func Decrypt(buf []byte, alg jwa.KeyEncryptionAlgorithm, key interface{}, option var dst *Message var postParse PostParser + // in v1 the default value is hardcoded. Use v2 if you want to change this value globally + var maxDecompressBufferSize int64 = 10 * 1024 * 1024 //nolint:forcetypeassert for _, option := range options { switch option.Ident() { @@ -227,6 +236,8 @@ func Decrypt(buf []byte, alg jwa.KeyEncryptionAlgorithm, key interface{}, option dst = option.Value().(*Message) case identPostParser{}: postParse = option.Value().(PostParser) + case identMaxDecompressBufferSize{}: + maxDecompressBufferSize = option.Value().(int64) } } @@ -241,9 +252,11 @@ func Decrypt(buf []byte, alg jwa.KeyEncryptionAlgorithm, key interface{}, option return nil, errors.Wrap(err, `failed to execute PostParser hook`) } } + ctx.maxDecompressBufferSize = maxDecompressBufferSize payload, err := doDecryptCtx(&ctx) if err != nil { + fmt.Printf("failed to decrypt: %s\n", err) return nil, errors.Wrap(err, `failed to decrypt message`) } diff --git a/jwe/jwe_test.go b/jwe/jwe_test.go index 8bf674899..40a3d08f7 100644 --- a/jwe/jwe_test.go +++ b/jwe/jwe_test.go @@ -21,6 +21,7 @@ import ( "github.com/lestrrat-go/jwx/jwk" "github.com/lestrrat-go/jwx/x25519" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -790,3 +791,66 @@ func TestGH554(t *testing.T) { return } } + +func TestMaxDecompressBufferSize(t *testing.T) { + // This payload size is intentionally set to a small value to avoid + // causing problems for regular users and CI/CD systems. If you wish to + // verify that root issue is fixed, you may want to try increasing the + // payload size to a larger value. + const payloadSize = 1 << 16 + + privkey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err, `rsa.GenerateKey should succeed`) + + pubkey := &privkey.PublicKey + + wrongPrivkey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err, `rsa.GenerateKey should succeed`) + wrongPubkey := &wrongPrivkey.PublicKey + + payload := strings.Repeat("x", payloadSize) + + testcases := []struct { + Name string + MaxSize int64 + PublicKey *rsa.PublicKey + Error bool + }{ + // This should work, because we set the MaxSize to be large (==payload size) + { + Name: "same as payload size", + MaxSize: payloadSize, + PublicKey: pubkey, + }, + // This should fail, because we set the GlobalMaxSize to be smaller than the payload size + { + Name: "smaller than payload size", + MaxSize: payloadSize - 1, + PublicKey: pubkey, + Error: true, + }, + // This should fail, because the public key does not match the + // private key used to decrypt the payload. In essence this way + // we do NOT trigger the root cause of this issue, but we bail out early + { + Name: "Wrong PublicKey", + PublicKey: wrongPubkey, + MaxSize: payloadSize, + Error: true, + }, + } + for _, tc := range testcases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + encrypted, err := jwe.Encrypt([]byte(payload), jwa.RSA_OAEP, tc.PublicKey, "A128CBC-HS256", jwa.Deflate) + require.NoError(t, err, `jwe.Encrypt should succeed`) + + _, err = jwe.Decrypt(encrypted, jwa.RSA_OAEP, privkey, jwe.WithMaxDecompressBufferSize(tc.MaxSize)) + if tc.Error { + require.Error(t, err, `jwe.Decrypt should fail`) + } else { + require.NoError(t, err, `jwe.Decrypt should succeed`) + } + }) + } +} diff --git a/jwe/message.go b/jwe/message.go index 9559877e3..d8fe660d6 100644 --- a/jwe/message.go +++ b/jwe/message.go @@ -632,9 +632,10 @@ func doDecryptCtx(dctx *decryptCtx) ([]byte, error) { } if h2.Compression() == jwa.Deflate { - buf, err := uncompress(plaintext) + buf, err := uncompress(plaintext, dctx.maxDecompressBufferSize) if err != nil { lastError = errors.Wrap(err, `failed to uncompress payload`) + plaintext = nil continue } plaintext = buf diff --git a/jwe/options.go b/jwe/options.go index 617e0e47c..9b33732be 100644 --- a/jwe/options.go +++ b/jwe/options.go @@ -11,6 +11,7 @@ type identMessage struct{} type identPostParser struct{} type identPrettyFormat struct{} type identProtectedHeader struct{} +type identMaxDecompressBufferSize struct{} type DecryptOption interface { Option @@ -23,6 +24,14 @@ type decryptOption struct { func (*decryptOption) decryptOption() {} +// WithMaxDecompressBufferSize specifies the maximum buffer size for used when +// decompressing the payload of a JWE message. If a JWE payload is compressed, +// and the size of the decompressed payload exceeds this amount, and error is +// returned. The default value is 10MB. +func WithMaxDecompressBufferSize(size int64) DecryptOption { + return &decryptOption{option.New(identMaxDecompressBufferSize{}, size)} +} + type SerializerOption interface { Option serializerOption()