From 5f32a1ce47b071086da90be7dd8a5285eb806ff0 Mon Sep 17 00:00:00 2001 From: 2rigor <39034718+2rigor@users.noreply.github.com> Date: Mon, 21 Oct 2024 16:14:10 +0300 Subject: [PATCH 1/4] make tar file limit configurable in build time add unit tests Signed-off-by: 2rigor <39034718+2rigor@users.noreply.github.com> --- pkg/file/tarutil.go | 19 +++++++++++++++++-- pkg/file/tarutil_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go index 3e1435c3..96dbfd45 100644 --- a/pkg/file/tarutil.go +++ b/pkg/file/tarutil.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "strconv" "strings" "github.com/spf13/afero" @@ -14,7 +15,9 @@ import ( "github.com/anchore/stereoscope/internal/log" ) -const perFileReadLimit = 2 * GB +const perFileReadLimitDefault = 2 * GB +var perFileReadLimit int64 = perFileReadLimitDefault +var perFileReadLimitStr = "0" // allows configuring the above value during build time var ErrTarStopIteration = fmt.Errorf("halt iterating tar") @@ -39,6 +42,18 @@ type ErrFileNotFound struct { Path string } +func init() { + setPerFileReadLimit(perFileReadLimitStr) +} + +func setPerFileReadLimit(val string) { + valInt64, err := strconv.ParseInt(val, 10, 64) + if err != nil || valInt64 <= 0 { + return + } + perFileReadLimit = valInt64 +} + func (e *ErrFileNotFound) Error() string { return fmt.Sprintf("file not found (path=%s)", e.Path) } @@ -178,7 +193,7 @@ func (v tarVisitor) visit(entry TarFileEntry) error { // limit the reader on each file read to prevent decompression bomb attacks numBytes, err := io.Copy(f, io.LimitReader(entry.Reader, perFileReadLimit)) if numBytes >= perFileReadLimit || errors.Is(err, io.EOF) { - return fmt.Errorf("zip read limit hit (potential decompression bomb attack)") + return fmt.Errorf("zip read limit hit (potential decompression bomb attack): copied %v, limit %v", numBytes, perFileReadLimit) } if err != nil { return fmt.Errorf("unable to copy file: %w", err) diff --git a/pkg/file/tarutil_test.go b/pkg/file/tarutil_test.go index 8320986b..db49aed6 100644 --- a/pkg/file/tarutil_test.go +++ b/pkg/file/tarutil_test.go @@ -430,3 +430,33 @@ func Test_tarVisitor_visit(t *testing.T) { }) } } + +func TestSetPerFileReadLimit_Valid(t *testing.T) { + // Test with a valid limit string + perFileReadLimit = perFileReadLimitDefault + setPerFileReadLimit("12345") + + if perFileReadLimit != 12345 { + t.Errorf("Expected perFileReadLimit to be 12345, but got %d", perFileReadLimit) + } +} + +func TestSetPerFileReadLimit_Invalid(t *testing.T) { + // Test with a invalid limit string + perFileReadLimit = perFileReadLimitDefault + setPerFileReadLimit("invalid") + + if perFileReadLimit != perFileReadLimitDefault { + t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) + } +} + +func TestSetPerFileReadLimit_Empty(t *testing.T) { + // Test with an empty limit string (default) + perFileReadLimit = perFileReadLimitDefault + setPerFileReadLimit("") + + if perFileReadLimit != 2 * GB { + t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) + } +} From 6ee403543ab390669f0bf4d4d15ce3fb4175854a Mon Sep 17 00:00:00 2001 From: 2rigor <39034718+2rigor@users.noreply.github.com> Date: Sun, 27 Oct 2024 17:44:02 +0200 Subject: [PATCH 2/4] replace 2GB by constant for the default size Signed-off-by: 2rigor <39034718+2rigor@users.noreply.github.com> --- pkg/file/tarutil_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/file/tarutil_test.go b/pkg/file/tarutil_test.go index db49aed6..e0157f71 100644 --- a/pkg/file/tarutil_test.go +++ b/pkg/file/tarutil_test.go @@ -433,7 +433,7 @@ func Test_tarVisitor_visit(t *testing.T) { func TestSetPerFileReadLimit_Valid(t *testing.T) { // Test with a valid limit string - perFileReadLimit = perFileReadLimitDefault + perFileReadLimit = perFileReadLimitDefault setPerFileReadLimit("12345") if perFileReadLimit != 12345 { @@ -443,7 +443,7 @@ func TestSetPerFileReadLimit_Valid(t *testing.T) { func TestSetPerFileReadLimit_Invalid(t *testing.T) { // Test with a invalid limit string - perFileReadLimit = perFileReadLimitDefault + perFileReadLimit = perFileReadLimitDefault setPerFileReadLimit("invalid") if perFileReadLimit != perFileReadLimitDefault { @@ -453,10 +453,10 @@ func TestSetPerFileReadLimit_Invalid(t *testing.T) { func TestSetPerFileReadLimit_Empty(t *testing.T) { // Test with an empty limit string (default) - perFileReadLimit = perFileReadLimitDefault + perFileReadLimit = perFileReadLimitDefault setPerFileReadLimit("") - if perFileReadLimit != 2 * GB { + if perFileReadLimit != perFileReadLimitDefault { t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) } } From 2b6e3caa3ce4fe3f3cb9cfb17e28a6cce2defd4b Mon Sep 17 00:00:00 2001 From: 2rigor <39034718+2rigor@users.noreply.github.com> Date: Wed, 30 Oct 2024 21:19:36 +0200 Subject: [PATCH 3/4] add Public setter, update unit tests Signed-off-by: 2rigor <39034718+2rigor@users.noreply.github.com> --- pkg/file/tarutil.go | 17 ++++++++----- pkg/file/tarutil_test.go | 52 ++++++++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go index 96dbfd45..269a9205 100644 --- a/pkg/file/tarutil.go +++ b/pkg/file/tarutil.go @@ -16,6 +16,7 @@ import ( ) const perFileReadLimitDefault = 2 * GB + var perFileReadLimit int64 = perFileReadLimitDefault var perFileReadLimitStr = "0" // allows configuring the above value during build time @@ -43,15 +44,19 @@ type ErrFileNotFound struct { } func init() { - setPerFileReadLimit(perFileReadLimitStr) + setPerFileReadLimitStr(perFileReadLimitStr) +} + +func setPerFileReadLimitStr(val string) { + if parsedInt64, err := strconv.ParseInt(val, 10, 64); err == nil { + SetPerFileReadLimit(parsedInt64) + } } -func setPerFileReadLimit(val string) { - valInt64, err := strconv.ParseInt(val, 10, 64) - if err != nil || valInt64 <= 0 { - return +func SetPerFileReadLimit(val int64) { + if val > 0 { + perFileReadLimit = val } - perFileReadLimit = valInt64 } func (e *ErrFileNotFound) Error() string { diff --git a/pkg/file/tarutil_test.go b/pkg/file/tarutil_test.go index e0157f71..a7cc5f51 100644 --- a/pkg/file/tarutil_test.go +++ b/pkg/file/tarutil_test.go @@ -432,31 +432,41 @@ func Test_tarVisitor_visit(t *testing.T) { } func TestSetPerFileReadLimit_Valid(t *testing.T) { - // Test with a valid limit string - perFileReadLimit = perFileReadLimitDefault - setPerFileReadLimit("12345") - - if perFileReadLimit != 12345 { - t.Errorf("Expected perFileReadLimit to be 12345, but got %d", perFileReadLimit) - } + // Test with a valid limit string + SetPerFileReadLimit(perFileReadLimitDefault) + setPerFileReadLimitStr("12345") + + if perFileReadLimit != 12345 { + t.Errorf("Expected perFileReadLimit to be 12345, but got %d", perFileReadLimit) + } } func TestSetPerFileReadLimit_Invalid(t *testing.T) { - // Test with a invalid limit string - perFileReadLimit = perFileReadLimitDefault - setPerFileReadLimit("invalid") - - if perFileReadLimit != perFileReadLimitDefault { - t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) - } + // Test with an invalid limit string + SetPerFileReadLimit(perFileReadLimitDefault) + setPerFileReadLimitStr("invalid") + + if perFileReadLimit != perFileReadLimitDefault { + t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) + } +} + +func TestSetPerFileReadLimit_NegativeValue(t *testing.T) { + // Test with an invalid limit string + SetPerFileReadLimit(perFileReadLimitDefault) + setPerFileReadLimitStr("-12345") + + if perFileReadLimit != perFileReadLimitDefault { + t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) + } } func TestSetPerFileReadLimit_Empty(t *testing.T) { - // Test with an empty limit string (default) - perFileReadLimit = perFileReadLimitDefault - setPerFileReadLimit("") - - if perFileReadLimit != perFileReadLimitDefault { - t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) - } + // Test with an empty limit string (default) + SetPerFileReadLimit(perFileReadLimitDefault) + setPerFileReadLimitStr("") + + if perFileReadLimit != perFileReadLimitDefault { + t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) + } } From 84a0127e98f158c29abd6763d04123ded4262276 Mon Sep 17 00:00:00 2001 From: 2rigor <39034718+2rigor@users.noreply.github.com> Date: Thu, 31 Oct 2024 11:56:29 +0200 Subject: [PATCH 4/4] leaving public setting only, remove unit tests Signed-off-by: 2rigor <39034718+2rigor@users.noreply.github.com> --- pkg/file/tarutil.go | 22 ++++------------------ pkg/file/tarutil_test.go | 40 ---------------------------------------- 2 files changed, 4 insertions(+), 58 deletions(-) diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go index 269a9205..32b7929b 100644 --- a/pkg/file/tarutil.go +++ b/pkg/file/tarutil.go @@ -7,7 +7,6 @@ import ( "io" "os" "path/filepath" - "strconv" "strings" "github.com/spf13/afero" @@ -15,10 +14,7 @@ import ( "github.com/anchore/stereoscope/internal/log" ) -const perFileReadLimitDefault = 2 * GB - -var perFileReadLimit int64 = perFileReadLimitDefault -var perFileReadLimitStr = "0" // allows configuring the above value during build time +var perFileReadLimit int64 = 2 * GB var ErrTarStopIteration = fmt.Errorf("halt iterating tar") @@ -43,19 +39,9 @@ type ErrFileNotFound struct { Path string } -func init() { - setPerFileReadLimitStr(perFileReadLimitStr) -} - -func setPerFileReadLimitStr(val string) { - if parsedInt64, err := strconv.ParseInt(val, 10, 64); err == nil { - SetPerFileReadLimit(parsedInt64) - } -} - -func SetPerFileReadLimit(val int64) { - if val > 0 { - perFileReadLimit = val +func SetPerFileReadLimit(maxBytes int64) { + if maxBytes > 0 { + perFileReadLimit = maxBytes } } diff --git a/pkg/file/tarutil_test.go b/pkg/file/tarutil_test.go index a7cc5f51..8320986b 100644 --- a/pkg/file/tarutil_test.go +++ b/pkg/file/tarutil_test.go @@ -430,43 +430,3 @@ func Test_tarVisitor_visit(t *testing.T) { }) } } - -func TestSetPerFileReadLimit_Valid(t *testing.T) { - // Test with a valid limit string - SetPerFileReadLimit(perFileReadLimitDefault) - setPerFileReadLimitStr("12345") - - if perFileReadLimit != 12345 { - t.Errorf("Expected perFileReadLimit to be 12345, but got %d", perFileReadLimit) - } -} - -func TestSetPerFileReadLimit_Invalid(t *testing.T) { - // Test with an invalid limit string - SetPerFileReadLimit(perFileReadLimitDefault) - setPerFileReadLimitStr("invalid") - - if perFileReadLimit != perFileReadLimitDefault { - t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) - } -} - -func TestSetPerFileReadLimit_NegativeValue(t *testing.T) { - // Test with an invalid limit string - SetPerFileReadLimit(perFileReadLimitDefault) - setPerFileReadLimitStr("-12345") - - if perFileReadLimit != perFileReadLimitDefault { - t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) - } -} - -func TestSetPerFileReadLimit_Empty(t *testing.T) { - // Test with an empty limit string (default) - SetPerFileReadLimit(perFileReadLimitDefault) - setPerFileReadLimitStr("") - - if perFileReadLimit != perFileReadLimitDefault { - t.Errorf("Expected perFileReadLimit to be %d, but got %d", perFileReadLimitDefault, perFileReadLimit) - } -}