From ad8c29d1628435cdad008b5cbbfbe75f5bfc74ae Mon Sep 17 00:00:00 2001 From: lestrrat <49281+lestrrat@users.noreply.github.com> Date: Mon, 23 May 2022 21:48:33 +0900 Subject: [PATCH] merge develop/v1 (#747) * Bump github.com/goccy/go-json from 0.9.6 to 0.9.7 (#710) * Bump github.com/goccy/go-json from 0.9.6 to 0.9.7 Bumps [github.com/goccy/go-json](https://github.com/goccy/go-json) from 0.9.6 to 0.9.7. - [Release notes](https://github.com/goccy/go-json/releases) - [Changelog](https://github.com/goccy/go-json/blob/master/CHANGELOG.md) - [Commits](https://github.com/goccy/go-json/compare/v0.9.6...v0.9.7) --- updated-dependencies: - dependency-name: github.com/goccy/go-json dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * make tidy Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daisuke Maki * Update golang.org/x/crypto (#726) * Update Changes * [jwe/v1] Fix possible excessive unpadding for AESCBC (#746) * Fix possible excessive unpadding for AESCBC * Update Changes * Update Changes * Update Changes Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Changes | 6 ++++++ jwe/internal/aescbc/aescbc.go | 31 ++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/Changes b/Changes index 86782cdc9..ee8879887 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,12 @@ Changes ======= +v1.2.25 23 May 2022 +[Bug Fixes][Security] + * [jwe] An old bug from at least 7 years ago existed in handling AES-CBC unpadding, + where the unpad operation might remove more bytes than necessary (#744) + This affects all jwx code that is available before v2.0.2 and v1.2.25. + v1.2.24 05 May 2022 [Security] * Upgrade golang.org/x/crypto (#724) diff --git a/jwe/internal/aescbc/aescbc.go b/jwe/internal/aescbc/aescbc.go index f08f2cbec..9163688e9 100644 --- a/jwe/internal/aescbc/aescbc.go +++ b/jwe/internal/aescbc/aescbc.go @@ -35,23 +35,36 @@ func pad(buf []byte, n int) []byte { func unpad(buf []byte, n int) ([]byte, error) { lbuf := len(buf) rem := lbuf % n + + // First, `buf` must be a multiple of `n` if rem != 0 { return nil, errors.Errorf("input buffer must be multiple of block size %d", n) } - count := 0 + // Find the last byte, which is the encoded padding + // i.e. 0x1 == 1 byte worth of padding last := buf[lbuf-1] - for i := lbuf - 1; i >= 0; i-- { - if buf[i] != last { - break - } - count++ + + // This is the number of padding bytes that we expect + expected := int(last) + + if expected == 0 || /* we _have_ to have padding here. therefore, 0x0 is not an option */ + expected > n || /* we also must make sure that we don't go over the block size (n) */ + expected > lbuf /* finally, it can't be more than the buffer itself. unlikely, but could happen */ { + return nil, fmt.Errorf(`invalid padding byte at the end of buffer`) } - if count != int(last) { - return nil, errors.New("invalid padding") + + // start i = 1 because we have already established that expected == int(last) where + // last = buf[lbuf-1]. + // + // we also don't check against lbuf-i in range, because we have established expected <= lbuf + for i := 1; i < expected; i++ { + if buf[lbuf-i] != last { + return nil, errors.New(`invalid padding`) + } } - return buf[:lbuf-int(last)], nil + return buf[:lbuf-expected], nil } type Hmac struct {