From d10f20c2336bd9253959a682ab76f0cb62fde21c Mon Sep 17 00:00:00 2001 From: Daniel Hobe Date: Thu, 21 May 2026 20:09:41 -0700 Subject: [PATCH] fix(lzma2): prevent undefined bit shift overflow in dictionary calculation - Validate that the LZMA2 dictionary property byte p[0] is <= 40 (maximum valid dictionary size of 4GB) in NewReader before calculating DictCap. - Prevents an undefined bit shift overflow during runtime calculation when p[0] > 40. - Source: https://en.wikipedia.org/wiki/LZMA#LZMA2_format --- internal/lzma2/reader.go | 5 ++++ internal/lzma2/reader_test.go | 44 +++++++++++++++++++++++++++++++++++ reader_test.go | 31 +++++++++++++----------- 3 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 internal/lzma2/reader_test.go diff --git a/internal/lzma2/reader.go b/internal/lzma2/reader.go index 3f2e7be..59cca7b 100644 --- a/internal/lzma2/reader.go +++ b/internal/lzma2/reader.go @@ -18,6 +18,7 @@ var ( errAlreadyClosed = errors.New("lzma2: already closed") errNeedOneReader = errors.New("lzma2: need exactly one reader") errInsufficientProperties = errors.New("lzma2: not enough properties") + errInvalidProperties = errors.New("lzma2: invalid properties") ) func (rc *readCloser) Close() error { @@ -57,6 +58,10 @@ func NewReader(p []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro return nil, errInsufficientProperties } + if p[0] > 40 { + return nil, errInvalidProperties + } + config := lzma.Reader2Config{ DictCap: (2 | (int(p[0]) & 1)) << (p[0]/2 + 11), // This gem came from Lzma2Dec.c } diff --git a/internal/lzma2/reader_test.go b/internal/lzma2/reader_test.go new file mode 100644 index 0000000..b6a743e --- /dev/null +++ b/internal/lzma2/reader_test.go @@ -0,0 +1,44 @@ +//nolint:paralleltest,testpackage +package lzma2 + +import ( + "bytes" + "errors" + "io" + "testing" +) + +type dummyReadCloser struct { + io.Reader +} + +func (d dummyReadCloser) Close() error { + return nil +} + +func TestNewReader(t *testing.T) { + // Test too many/few readers + _, err := NewReader([]byte{0}, 0, nil) + if !errors.Is(err, errNeedOneReader) { + t.Errorf("expected errNeedOneReader, got %v", err) + } + + // Test invalid property length + _, err = NewReader([]byte{}, 0, []io.ReadCloser{dummyReadCloser{bytes.NewReader(nil)}}) + if !errors.Is(err, errInsufficientProperties) { + t.Errorf("expected errInsufficientProperties, got %v", err) + } + + // Test invalid property byte (> 40) + _, err = NewReader([]byte{41}, 0, []io.ReadCloser{dummyReadCloser{bytes.NewReader(nil)}}) + if !errors.Is(err, errInvalidProperties) { + t.Errorf("expected errInvalidProperties, got %v", err) + } + + // Test valid property byte (<= 40) + // Any value <= 40 should pass the property byte check + _, err = NewReader([]byte{0}, 0, []io.ReadCloser{dummyReadCloser{bytes.NewReader(nil)}}) + if errors.Is(err, errInvalidProperties) { + t.Errorf("unexpected errInvalidProperties for valid property, got %v", err) + } +} diff --git a/reader_test.go b/reader_test.go index 2f7a3f4..771ba1f 100644 --- a/reader_test.go +++ b/reader_test.go @@ -1,4 +1,3 @@ -//nolint:goconst package sevenzip_test import ( @@ -23,6 +22,12 @@ import ( "golang.org/x/sync/errgroup" ) +const ( + testNoHeaderCompression = "no header compression" + testPassword = "password" + testT07z = "t0.7z" +) + func reader(r io.Reader) io.Reader { return r } @@ -95,8 +100,8 @@ func TestOpenReader(t *testing.T) { err error }{ { - name: "no header compression", - file: "t0.7z", + name: testNoHeaderCompression, + file: testT07z, }, { name: "with header compression", @@ -254,24 +259,24 @@ func TestOpenReaderWithPassword(t *testing.T) { name, file, password string }{ { - name: "no header compression", + name: testNoHeaderCompression, file: "t2.7z", - password: "password", + password: testPassword, }, { name: "with header compression", file: "t3.7z", - password: "password", + password: testPassword, }, { name: "unencrypted headers compressed files", file: "t4.7z", - password: "password", + password: testPassword, }, { name: "unencrypted headers uncompressed files", file: "t5.7z", - password: "password", + password: testPassword, }, { name: "issue 75", @@ -358,12 +363,12 @@ func TestNewReader(t *testing.T) { err error }{ { - name: "no header compression", - file: "t0.7z", + name: testNoHeaderCompression, + file: testT07z, }, { - name: "no header compression", - file: "t0.7z", + name: testNoHeaderCompression, + file: testT07z, size: -1, err: sevenzip.ErrNegativeSize, }, @@ -589,7 +594,7 @@ func benchmarkArchive(b *testing.B, file, password string, optimised bool) { } func BenchmarkAES7z(b *testing.B) { - benchmarkArchive(b, "aes7z.7z", "password", true) + benchmarkArchive(b, "aes7z.7z", testPassword, true) } func BenchmarkBzip2(b *testing.B) {