Skip to content

Commit 6af6e80

Browse files
committed
fix: address fuzzing findings and other miscellaneous issues
Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>
1 parent 2bf11c5 commit 6af6e80

48 files changed

Lines changed: 703 additions & 224 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ GLOBAL OPTIONS:
6464
--ignore-tags string Rule tags to ignore (default: "false_positive,ignore")
6565
--include-data-files Include files that are detected as non-program (binary or source) files
6666
--jobs int, -j int Concurrently scan files within target scan paths (default: 12)
67-
--max-depth int Maximum depth for archive extraction (-1 for unlimited) (default: 32)
68-
--max-files int Maximum number of files to scan (-1 for unlimited) (default: 2097152)
67+
--max-depth int Maximum depth for archive extraction (0 or -1 for unlimited) (default: 32)
68+
--max-files int Maximum number of files to scan (0 or -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: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"runtime"
1616
"strings"
1717
"sync"
18-
"time"
1918

2019
"github.com/chainguard-dev/clog"
2120
"github.com/chainguard-dev/malcontent/pkg/file"
@@ -33,6 +32,34 @@ func init() {
3332
zipPool = pool.NewBufferPool(runtime.GOMAXPROCS(0) * 2)
3433
}
3534

35+
// ValidateResolvedPath checks that the target path still resides within the extraction directory
36+
// after resolving symlinks in its parent directory.
37+
func ValidateResolvedPath(target, dir, clean string) error {
38+
resolvedParent, ok := evalSymlinks(filepath.Dir(target))
39+
if !ok {
40+
return nil
41+
}
42+
resolvedDir, ok := evalSymlinks(dir)
43+
if !ok {
44+
return nil
45+
}
46+
resolvedTarget := filepath.Join(resolvedParent, filepath.Base(target))
47+
if !IsValidPath(resolvedTarget, resolvedDir) {
48+
return fmt.Errorf("path traversal via symlink in parent directory: %s", clean)
49+
}
50+
return nil
51+
}
52+
53+
// evalSymlinks resolves symlinks in the given path, returning the resolved path
54+
// and true on success, or an empty string and false if resolution fails.
55+
func evalSymlinks(path string) (string, bool) {
56+
resolved, err := filepath.EvalSymlinks(path)
57+
if err != nil {
58+
return "", false
59+
}
60+
return resolved, true
61+
}
62+
3663
// isValidPath checks if the target file is within the given directory.
3764
func IsValidPath(target, dir string) bool {
3865
if strings.Contains(target, "\x00") || strings.Contains(dir, "\x00") {
@@ -137,13 +164,15 @@ func extractNestedArchive(ctx context.Context, c malcontent.Config, d string, f
137164
// Some packages may have archives and files with colliding names
138165
// e.g., demo_page.css and demo_page.css.gz
139166
// the former is the uncompressed version of the latter
140-
// if we encounter this, replace the name with something that won't collide
167+
// if we encounter this, use os.MkdirTemp to create a unique directory
141168
if _, err := os.Stat(archivePath); err == nil {
142169
logger.Debugf("duplicate file name already exists, modifying directory name for %s", archivePath)
143-
archivePath = fmt.Sprintf("%s%d", archivePath, time.Now().UnixNano())
144-
}
145-
146-
if err := os.MkdirAll(archivePath, 0o700); err != nil {
170+
var mkErr error
171+
archivePath, mkErr = os.MkdirTemp(filepath.Dir(archivePath), filepath.Base(archivePath)+"_*")
172+
if mkErr != nil {
173+
return fmt.Errorf("failed to create unique extraction directory: %w", mkErr)
174+
}
175+
} else if err := os.MkdirAll(archivePath, 0o700); err != nil {
147176
return fmt.Errorf("failed to create extraction directory: %w", err)
148177
}
149178

@@ -331,9 +360,19 @@ func handleSymlink(dir, linkPath, linkTarget string) error {
331360
return nil
332361
}
333362

363+
parentDir := filepath.Dir(fullPath)
364+
resolvedDir := dir
365+
if rp, err := filepath.EvalSymlinks(parentDir); err == nil {
366+
parentDir = rp
367+
if rd, err := filepath.EvalSymlinks(dir); err == nil {
368+
resolvedDir = rd
369+
}
370+
}
371+
334372
// Validate relative symlink target resolves within extraction directory
335-
resolvedTarget := filepath.Clean(filepath.Join(filepath.Dir(fullPath), linkTarget))
336-
if !IsValidPath(resolvedTarget, dir) {
373+
// using the actual (resolved) parent directory
374+
resolvedTarget := filepath.Clean(filepath.Join(parentDir, linkTarget))
375+
if !IsValidPath(resolvedTarget, resolvedDir) {
337376
return fmt.Errorf("symlink target escapes extraction directory: %s -> %s", linkPath, linkTarget)
338377
}
339378

@@ -363,8 +402,9 @@ func handleSymlink(dir, linkPath, linkTarget string) error {
363402
return fmt.Errorf("symlink target mismatch: expected %s, got %s", linkTarget, actualTarget)
364403
}
365404

366-
actualResolved := filepath.Clean(filepath.Join(filepath.Dir(fullPath), actualTarget))
367-
if !IsValidPath(actualResolved, dir) {
405+
// Post-creation validation using the resolved parent directory
406+
actualResolved := filepath.Clean(filepath.Join(parentDir, actualTarget))
407+
if !IsValidPath(actualResolved, resolvedDir) {
368408
os.Remove(fullPath)
369409
return fmt.Errorf("symlink target escapes extraction directory after creation: %s -> %s", linkPath, actualTarget)
370410
}

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: 11 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
}
@@ -57,6 +63,10 @@ func ExtractDeb(ctx context.Context, d, f string) error {
5763
return fmt.Errorf("invalid file path: %s", target)
5864
}
5965

66+
if err := ValidateResolvedPath(target, d, clean); err != nil {
67+
return err
68+
}
69+
6070
switch header.Typeflag {
6171
case tar.TypeDir:
6272
if err := handleDirectory(target); err != nil {

0 commit comments

Comments
 (0)