Skip to content

Commit 5540268

Browse files
committed
more improvements
Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>
1 parent 820fd2e commit 5540268

20 files changed

Lines changed: 142 additions & 69 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ GLOBAL OPTIONS:
6666
--jobs int, -j int Concurrently scan files within target scan paths (default: 12)
6767
--max-depth int Maximum depth for archive extraction (-1 for unlimited) (default: 32)
6868
--max-files int Maximum number of files to scan (-1 for unlimited) (default: 2097152)
69+
--max-image-size int Maximum OCI image size in bytes (0 or -1 for unlimited) (default: 17179869184)
6970
--min-file-level int Obsoleted by --min-file-risk (default: -1)
7071
--min-file-risk string Only show results for files which meet the given risk level (any, low, medium, high, critical) (default: "low")
7172
--min-level int Obsoleted by --min-risk (default: -1)

cmd/mal/mal.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ var (
6161
ignoreTagsFlag string
6262
includeDataFilesFlag bool
6363
maxDepthFlag int
64+
maxImageSizeFlag int64
6465
maxScanFilesFlag int
6566
minFileLevelFlag int
6667
minFileRiskFlag string
@@ -261,6 +262,7 @@ func main() {
261262
IgnoreTags: ignoreTags,
262263
IncludeDataFiles: includeDataFiles,
263264
MaxDepth: maxDepthFlag,
265+
MaxImageSize: maxImageSizeFlag,
264266
MaxScanFiles: maxScanFilesFlag,
265267
MinFileRisk: minFileRisk,
266268
MinRisk: minRisk,
@@ -348,17 +350,24 @@ func main() {
348350
&cli.IntFlag{
349351
Name: "max-depth",
350352
Value: 32,
351-
Usage: "Maximum depth for archive extraction (-1 for unlimited)",
353+
Usage: "Maximum depth for archive extraction (0 or -1 for unlimited)",
352354
Destination: &maxDepthFlag,
353355
Local: false,
354356
},
355357
&cli.IntFlag{
356358
Name: "max-files",
357359
Value: 1 << 21, // ~2 million files
358-
Usage: "Maximum number of files to scan (-1 for unlimited)",
360+
Usage: "Maximum number of files to scan (0 or -1 for unlimited)",
359361
Destination: &maxScanFilesFlag,
360362
Local: false,
361363
},
364+
&cli.Int64Flag{
365+
Name: "max-image-size",
366+
Value: 1 << 34, // ~16 GB
367+
Usage: "Maximum OCI image size in bytes (0 or -1 for unlimited)",
368+
Destination: &maxImageSizeFlag,
369+
Local: false,
370+
},
362371
&cli.IntFlag{
363372
Name: "min-file-level",
364373
Value: -1,

pkg/action/archive_test.go

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,30 @@ import (
1515

1616
"github.com/chainguard-dev/clog"
1717
"github.com/chainguard-dev/malcontent/pkg/archive"
18+
"github.com/chainguard-dev/malcontent/pkg/file"
1819
"github.com/chainguard-dev/malcontent/pkg/malcontent"
1920
"github.com/chainguard-dev/malcontent/pkg/render"
2021
"github.com/chainguard-dev/malcontent/rules"
2122
thirdparty "github.com/chainguard-dev/malcontent/third_party"
2223
"github.com/google/go-cmp/cmp"
2324
)
2425

26+
// readTestFile reads a file using file.GetContents for consistency with production code.
27+
func readTestFile(t *testing.T, path string) []byte {
28+
t.Helper()
29+
f, err := os.Open(path)
30+
if err != nil {
31+
t.Fatalf("failed to open test file %s: %v", path, err)
32+
}
33+
defer f.Close()
34+
buf := make([]byte, file.ExtractBuffer)
35+
data, err := file.GetContents(f, buf)
36+
if err != nil {
37+
t.Fatalf("failed to read test file %s: %v", path, err)
38+
}
39+
return data
40+
}
41+
2542
func TestExtractionMethod(t *testing.T) {
2643
tests := []struct {
2744
name string
@@ -256,10 +273,7 @@ func TestScanArchive(t *testing.T) {
256273

257274
got := out.String()
258275

259-
td, err := os.ReadFile("testdata/scan_archive")
260-
if err != nil {
261-
t.Fatalf("testdata read failed: %v", err)
262-
}
276+
td := readTestFile(t, "testdata/scan_archive")
263277
want := string(td)
264278

265279
if diff := cmp.Diff(want, got); diff != "" {
@@ -303,10 +317,7 @@ func TestScanDeb(t *testing.T) {
303317

304318
got := out.String()
305319

306-
td, err := os.ReadFile("testdata/scan_deb")
307-
if err != nil {
308-
t.Fatalf("testdata read failed: %v", err)
309-
}
320+
td := readTestFile(t, "testdata/scan_deb")
310321
want := string(td)
311322

312323
if diff := cmp.Diff(want, got); diff != "" {
@@ -350,10 +361,7 @@ func TestScanRPM(t *testing.T) {
350361

351362
got := out.String()
352363

353-
td, err := os.ReadFile("testdata/scan_rpm")
354-
if err != nil {
355-
t.Fatalf("testdata read failed: %v", err)
356-
}
364+
td := readTestFile(t, "testdata/scan_rpm")
357365
want := string(td)
358366

359367
if diff := cmp.Diff(want, got); diff != "" {
@@ -397,10 +405,7 @@ func TestScanZlib(t *testing.T) {
397405

398406
got := out.String()
399407

400-
td, err := os.ReadFile("testdata/scan_zlib")
401-
if err != nil {
402-
t.Fatalf("testdata read failed: %v", err)
403-
}
408+
td := readTestFile(t, "testdata/scan_zlib")
404409
want := string(td)
405410

406411
if diff := cmp.Diff(want, got); diff != "" {
@@ -444,10 +449,7 @@ func TestScanZstd(t *testing.T) {
444449

445450
got := out.String()
446451

447-
td, err := os.ReadFile("testdata/scan_zstd")
448-
if err != nil {
449-
t.Fatalf("testdata read failed: %v", err)
450-
}
452+
td := readTestFile(t, "testdata/scan_zstd")
451453
want := string(td)
452454

453455
if diff := cmp.Diff(want, got); diff != "" {
@@ -582,10 +584,7 @@ func TestScanConflictingArchiveFiles(t *testing.T) {
582584
}
583585

584586
got := out.String()
585-
td, err := os.ReadFile("testdata/scan_conflict")
586-
if err != nil {
587-
t.Fatalf("testdata read failed: %v", err)
588-
}
587+
td := readTestFile(t, "testdata/scan_conflict")
589588
want := string(td)
590589

591590
if diff := cmp.Diff(want, got); diff != "" {

pkg/action/diff.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,11 @@ func Diff(ctx context.Context, c malcontent.Config, _ *clog.Logger) (*malcontent
221221
)
222222

223223
if c.OCI {
224-
srcPath, err = archive.OCI(ctx, srcPath, c.OCIAuth)
224+
srcPath, err = archive.OCI(ctx, srcPath, c.OCIAuth, c.MaxImageSize)
225225
if err != nil {
226226
return nil, fmt.Errorf("failed to prepare scan path: %w", err)
227227
}
228-
destPath, err = archive.OCI(ctx, destPath, c.OCIAuth)
228+
destPath, err = archive.OCI(ctx, destPath, c.OCIAuth, c.MaxImageSize)
229229
if err != nil {
230230
return nil, fmt.Errorf("failed to prepare scan path: %w", err)
231231
}

pkg/action/oci_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"bytes"
88
"context"
99
"io/fs"
10-
"os"
1110
"runtime"
1211
"testing"
1312

@@ -55,10 +54,7 @@ func TestOCI(t *testing.T) {
5554

5655
got := out.String()
5756

58-
td, err := os.ReadFile("testdata/scan_oci")
59-
if err != nil {
60-
t.Fatalf("testdata read failed: %v", err)
61-
}
57+
td := readTestFile(t, "testdata/scan_oci")
6258
want := string(td)
6359

6460
if diff := cmp.Diff(want, got); diff != "" {

pkg/action/scan.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ func handleScanPath(ctx context.Context, scanPath string, c malcontent.Config, r
355355
c.Renderer.Scanning(ctx, scanPath)
356356
}
357357

358-
scanInfo, err := prepareScanPath(ctx, scanPath, c.OCI, c.OCIAuth, logger)
358+
scanInfo, err := prepareScanPath(ctx, scanPath, c.OCI, c.OCIAuth, c.MaxImageSize, logger)
359359
if err != nil {
360360
return fmt.Errorf("failed to prepare scan path: %w", err)
361361
}
@@ -376,7 +376,7 @@ func handleScanPath(ctx context.Context, scanPath string, c malcontent.Config, r
376376
return processPaths(ctx, paths, scanInfo, c, r, matchChan, matchOnce, logger)
377377
}
378378

379-
func prepareScanPath(ctx context.Context, scanPath string, isOCI, useAuth bool, logger *clog.Logger) (scanPathInfo, error) {
379+
func prepareScanPath(ctx context.Context, scanPath string, isOCI, useAuth bool, maxImageSize int64, logger *clog.Logger) (scanPathInfo, error) {
380380
if ctx.Err() != nil {
381381
return scanPathInfo{}, ctx.Err()
382382
}
@@ -391,7 +391,7 @@ func prepareScanPath(ctx context.Context, scanPath string, isOCI, useAuth bool,
391391
}
392392

393393
info.imageURI = scanPath
394-
ociPath, err := archive.OCI(ctx, info.imageURI, useAuth)
394+
ociPath, err := archive.OCI(ctx, info.imageURI, useAuth, maxImageSize)
395395
if err != nil {
396396
return info, fmt.Errorf("failed to prepare OCI image for scanning: %w", err)
397397
}

pkg/archive/archive.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ package archive
66
import (
77
"archive/tar"
88
"context"
9+
"crypto/rand"
10+
"encoding/hex"
911
"errors"
1012
"fmt"
1113
"io"
@@ -15,7 +17,6 @@ import (
1517
"runtime"
1618
"strings"
1719
"sync"
18-
"time"
1920

2021
"github.com/chainguard-dev/clog"
2122
"github.com/chainguard-dev/malcontent/pkg/file"
@@ -140,7 +141,9 @@ func extractNestedArchive(ctx context.Context, c malcontent.Config, d string, f
140141
// if we encounter this, replace the name with something that won't collide
141142
if _, err := os.Stat(archivePath); err == nil {
142143
logger.Debugf("duplicate file name already exists, modifying directory name for %s", archivePath)
143-
archivePath = fmt.Sprintf("%s%d", archivePath, time.Now().UnixNano())
144+
var suffix [8]byte
145+
_, _ = rand.Read(suffix[:])
146+
archivePath = fmt.Sprintf("%s_%s", archivePath, hex.EncodeToString(suffix[:]))
144147
}
145148

146149
if err := os.MkdirAll(archivePath, 0o700); err != nil {

pkg/archive/bz2.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ func ExtractBz2(ctx context.Context, d, f string) error {
6161
return fmt.Errorf("failed to create directory for file: %w", err)
6262
}
6363

64-
// #nosec G115 // ignore Type conversion which leads to integer overflow
65-
// header.Mode is int64 and FileMode is uint32
6664
out, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
6765
if err != nil {
6866
return fmt.Errorf("failed to create file: %w", err)

pkg/archive/deb.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,13 @@ import (
1818
)
1919

2020
// ExtractDeb extracts .deb packages.
21-
func ExtractDeb(ctx context.Context, d, f string) error {
21+
func ExtractDeb(ctx context.Context, d, f string) (retErr error) {
22+
// Recover from panics in third-party deb parsing library.
23+
defer func() {
24+
if r := recover(); r != nil {
25+
retErr = fmt.Errorf("recovered from panic during deb extraction: %v", r)
26+
}
27+
}()
2228
if ctx.Err() != nil {
2329
return ctx.Err()
2430
}

pkg/archive/fuzz_test.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,22 @@ import (
1212
"testing"
1313
"time"
1414

15+
"github.com/chainguard-dev/malcontent/pkg/file"
1516
"github.com/chainguard-dev/malcontent/pkg/malcontent"
1617
"github.com/chainguard-dev/malcontent/pkg/programkind"
1718
)
1819

20+
// readTestFile reads a file using file.GetContents for consistency with production code.
21+
func readTestFile(path string) ([]byte, error) {
22+
f, err := os.Open(path)
23+
if err != nil {
24+
return nil, err
25+
}
26+
defer f.Close()
27+
buf := make([]byte, file.ExtractBuffer)
28+
return file.GetContents(f, buf)
29+
}
30+
1931
// FuzzExtractTar tests tar extraction with random inputs to find crashes,
2032
// path traversal vulnerabilities, and other issues.
2133
func FuzzExtractTar(f *testing.F) {
@@ -30,7 +42,7 @@ func FuzzExtractTar(f *testing.F) {
3042
}
3143

3244
for _, td := range testdata {
33-
if data, err := os.ReadFile(td); err == nil {
45+
if data, err := readTestFile(td); err == nil {
3446
f.Add(data)
3547
}
3648
}
@@ -85,7 +97,7 @@ func FuzzExtractZip(f *testing.F) {
8597
}
8698

8799
for _, td := range testdata {
88-
if data, err := os.ReadFile(td); err == nil {
100+
if data, err := readTestFile(td); err == nil {
89101
f.Add(data)
90102
}
91103
}
@@ -152,7 +164,7 @@ func FuzzExtractArchive(f *testing.F) {
152164
}
153165

154166
for _, td := range testdata {
155-
if data, err := os.ReadFile(td); err == nil {
167+
if data, err := readTestFile(td); err == nil {
156168
switch {
157169
case strings.HasSuffix(td, ".tar.gz"):
158170
f.Add(data, ".tar.gz")
@@ -270,7 +282,7 @@ func FuzzExtractGzip(f *testing.F) {
270282
}
271283

272284
for _, td := range testdata {
273-
if data, err := os.ReadFile(td); err == nil {
285+
if data, err := readTestFile(td); err == nil {
274286
f.Add(data)
275287
}
276288
}
@@ -367,7 +379,7 @@ func FuzzExtractZstd(f *testing.F) {
367379
}
368380

369381
for _, td := range testdata {
370-
if data, err := os.ReadFile(td); err == nil {
382+
if data, err := readTestFile(td); err == nil {
371383
f.Add(data)
372384
}
373385
}
@@ -419,7 +431,7 @@ func FuzzExtractZlib(f *testing.F) {
419431
}
420432

421433
for _, td := range testdata {
422-
if data, err := os.ReadFile(td); err == nil {
434+
if data, err := readTestFile(td); err == nil {
423435
f.Add(data)
424436
}
425437
}
@@ -473,7 +485,7 @@ func FuzzExtractRPM(f *testing.F) {
473485
}
474486

475487
for _, td := range testdata {
476-
if data, err := os.ReadFile(td); err == nil {
488+
if data, err := readTestFile(td); err == nil {
477489
f.Add(data)
478490
}
479491
}
@@ -530,7 +542,7 @@ func FuzzExtractDeb(f *testing.F) {
530542
}
531543

532544
for _, td := range testdata {
533-
if data, err := os.ReadFile(td); err == nil {
545+
if data, err := readTestFile(td); err == nil {
534546
f.Add(data)
535547
}
536548
}

0 commit comments

Comments
 (0)