From e05f04ef08d075c764d03623bfcbf774566883a9 Mon Sep 17 00:00:00 2001 From: Madan Kumar Date: Thu, 1 Oct 2026 12:48:17 +0530 Subject: [PATCH] fix(encryption): return an error instead of panicking on a short GCM ciphertext (#3527) * fix(encryption): return an error instead of panicking on a short GCM ciphertext gcmCipher.Decrypt slices ciphertext[:nonceSize] without first checking the length, so a ciphertext shorter than the 12-byte GCM nonce triggers a slice-bounds-out-of-range panic instead of returning an error. The sibling cfbCipher.Decrypt already guards its IV length and returns a descriptive error; mirror that guard for GCM so decrypting a malformed (e.g. truncated) cookie fails cleanly. Adds a test covering both ciphers. Signed-off-by: Madan Kumar * docs(changelog): add entry for the GCM short-ciphertext fix Signed-off-by: Madan Kumar --------- Signed-off-by: Madan Kumar --- CHANGELOG.md | 2 ++ pkg/encryption/cipher.go | 3 +++ pkg/encryption/cipher_test.go | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c240122..985a408f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,12 @@ Additionally refer to OAuth client configuration for Bitbucket provider in the [ ## Changes since v7.15.4 +<<<<<<< HEAD - [#3546](https://github.com/oauth2-proxy/oauth2-proxy/pull/3546) fix: strip the port from the request host when matching cookie domains @kirilju - [#3547](https://github.com/oauth2-proxy/oauth2-proxy/pull/3547) fix: refresh additional claims for OIDC and MS Entra ID providers and properly populate additional claims during login (@Apollo3zehn) - [#3477](https://github.com/oauth2-proxy/oauth2-proxy/pull/3477) fix(bitbucket): auth failure due to Bitbucket OAuth 2.0 [changes on May 4th 2026](https://developer.atlassian.com/cloud/bitbucket/changelog/#CHANGE-3052) @mfouad - [#3535](https://github.com/oauth2-proxy/oauth2-proxy/pull/3535) fix: surface Microsoft Graph errors during Entra group overage instead of logging in with an incomplete group set @no-hup +- [#3527](https://github.com/oauth2-proxy/oauth2-proxy/pull/3527) fix(encryption): return an error instead of panicking on a short GCM ciphertext @winklemad # V7.15.4 diff --git a/pkg/encryption/cipher.go b/pkg/encryption/cipher.go index 300bba3a..eb13c5b3 100644 --- a/pkg/encryption/cipher.go +++ b/pkg/encryption/cipher.go @@ -122,6 +122,9 @@ func (c *gcmCipher) Decrypt(ciphertext []byte) ([]byte, error) { } nonceSize := gcm.NonceSize() + if len(ciphertext) < nonceSize { + return nil, fmt.Errorf("encrypted value should be at least %d bytes, but is only %d bytes", nonceSize, len(ciphertext)) + } nonce, ciphertext := ciphertext[:nonceSize], ciphertext[nonceSize:] plaintext, err := gcm.Open(nil, nonce, ciphertext, nil) diff --git a/pkg/encryption/cipher_test.go b/pkg/encryption/cipher_test.go index 16e12929..9499a5f2 100644 --- a/pkg/encryption/cipher_test.go +++ b/pkg/encryption/cipher_test.go @@ -89,6 +89,24 @@ func TestEncryptAndDecrypt(t *testing.T) { } } +func TestDecryptShortCiphertextReturnsError(t *testing.T) { + // A ciphertext shorter than the cipher's IV/nonce prefix must yield an + // error, not a slice-bounds panic. CFB already guarded this; GCM must too. + cipherInits := map[string]func([]byte) (Cipher, error){ + "CFB": NewCFBCipher, + "GCM": NewGCMCipher, + } + for name, initCipher := range cipherInits { + t.Run(name, func(t *testing.T) { + c, err := initCipher([]byte("0123456789abcdef")) + assert.NoError(t, err) + + _, err = c.Decrypt([]byte{1, 2, 3, 4, 5}) + assert.Error(t, err) + }) + } +} + func runEncryptAndDecrypt(t *testing.T, c Cipher, dataSize int) { data := make([]byte, dataSize) _, err := io.ReadFull(rand.Reader, data)