Skip to content

Commit c348e4b

Browse files
authored
Avoid failing scans outright when encountering extraction failures (#962)
* Avoid failing scans outright when encountering extraction failures Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> * Address gap between mimetype.Detect and Path re: archives Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> * Appease the linter Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> * Fix tests Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> * Add MIME type for 7z Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> --------- Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>
1 parent 5a4eed8 commit c348e4b

15 files changed

Lines changed: 861 additions & 10 deletions

File tree

cmd/mal/mal.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ var (
5050
allFlag bool
5151
concurrencyFlag int
5252
diffImageFlag bool
53+
exitExtractionFlag bool
5354
exitFirstHitFlag bool
5455
exitFirstMissFlag bool
5556
fileRiskChangeFlag bool
@@ -262,6 +263,7 @@ func main() {
262263

263264
mc = malcontent.Config{
264265
Concurrency: concurrency,
266+
ExitExtraction: exitExtractionFlag,
265267
ExitFirstHit: exitFirstHitFlag,
266268
ExitFirstMiss: exitFirstMissFlag,
267269
IgnoreSelf: ignoreSelfFlag,
@@ -287,6 +289,12 @@ func main() {
287289
Usage: "Ignore nothing within a provided scan path",
288290
Destination: &allFlag,
289291
},
292+
&cli.BoolFlag{
293+
Name: "exit-extraction",
294+
Value: true,
295+
Usage: "Exit when encountering file extraction errors",
296+
Destination: &exitExtractionFlag,
297+
},
290298
&cli.BoolFlag{
291299
Name: "exit-first-miss",
292300
Value: false,

pkg/action/archive_test.go

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"testing"
1212

1313
"github.com/chainguard-dev/clog"
14-
"github.com/chainguard-dev/clog/slogtest"
1514
"github.com/chainguard-dev/malcontent/pkg/archive"
1615
"github.com/chainguard-dev/malcontent/pkg/malcontent"
1716
"github.com/chainguard-dev/malcontent/pkg/programkind"
@@ -215,7 +214,7 @@ func TestExtractNestedArchive(t *testing.T) {
215214

216215
func TestScanArchive(t *testing.T) {
217216
t.Parallel()
218-
ctx := slogtest.Context(t)
217+
ctx := context.Background()
219218
clog.FromContext(ctx).With("test", "scan_archive")
220219

221220
var out bytes.Buffer
@@ -260,6 +259,95 @@ func TestScanArchive(t *testing.T) {
260259
}
261260
}
262261

262+
func extractError(e error) error {
263+
if strings.Contains(e.Error(), "not a valid gzip archive") || strings.Contains(e.Error(), "not a valid zip file") {
264+
return nil
265+
}
266+
return e
267+
}
268+
269+
func TestScanInvalidArchive(t *testing.T) {
270+
t.Parallel()
271+
ctx := context.Background()
272+
clog.FromContext(ctx).With("test", "scan_invalid_archive")
273+
274+
var out bytes.Buffer
275+
r, err := render.New("json", &out)
276+
if err != nil {
277+
t.Fatalf("render: %v", err)
278+
}
279+
280+
rfs := []fs.FS{rules.FS, thirdparty.FS}
281+
yrs, err := CachedRules(ctx, rfs)
282+
if err != nil {
283+
t.Fatalf("rules: %v", err)
284+
}
285+
286+
mc := malcontent.Config{
287+
Concurrency: runtime.NumCPU(),
288+
ExitExtraction: true,
289+
IgnoreSelf: false,
290+
MinFileRisk: 0,
291+
MinRisk: 0,
292+
Renderer: r,
293+
Rules: yrs,
294+
ScanPaths: []string{
295+
"testdata/17419.zip",
296+
"testdata/joblib_0.9.4.dev0_compressed_cache_size_pickle_py35_np19.gz",
297+
},
298+
}
299+
_, err = Scan(ctx, mc)
300+
err = extractError(err)
301+
if err != nil {
302+
t.Fatal(err)
303+
}
304+
}
305+
306+
func TestScanInvalidArchiveIgnore(t *testing.T) {
307+
t.Parallel()
308+
ctx := context.Background()
309+
clog.FromContext(ctx).With("test", "scan_invalid_archive_ignore")
310+
311+
var out bytes.Buffer
312+
r, err := render.New("json", &out)
313+
if err != nil {
314+
t.Fatalf("render: %v", err)
315+
}
316+
317+
rfs := []fs.FS{rules.FS, thirdparty.FS}
318+
yrs, err := CachedRules(ctx, rfs)
319+
if err != nil {
320+
t.Fatalf("rules: %v", err)
321+
}
322+
323+
mc := malcontent.Config{
324+
Concurrency: runtime.NumCPU(),
325+
ExitExtraction: false,
326+
IgnoreSelf: false,
327+
MinFileRisk: 0,
328+
MinRisk: 0,
329+
Renderer: r,
330+
Rules: yrs,
331+
ScanPaths: []string{
332+
"testdata/17419.zip",
333+
"testdata/joblib_0.9.4.dev0_compressed_cache_size_pickle_py35_np19.gz",
334+
},
335+
}
336+
res, err := Scan(ctx, mc)
337+
if err != nil {
338+
t.Fatal(err)
339+
}
340+
if err := r.Full(ctx, nil, res); err != nil {
341+
t.Fatalf("full: %v", err)
342+
}
343+
344+
got := out.String()
345+
want := "{}\n"
346+
if diff := cmp.Diff(want, got); diff != "" {
347+
t.Errorf("output mismatch: (-want +got):\n%s", diff)
348+
}
349+
}
350+
263351
func TestGetExt(t *testing.T) {
264352
tests := []struct {
265353
path string

pkg/action/oci_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ package action
22

33
import (
44
"bytes"
5+
"context"
56
"io/fs"
67
"os"
78
"runtime"
89
"testing"
910

1011
"github.com/chainguard-dev/clog"
11-
"github.com/chainguard-dev/clog/slogtest"
1212
"github.com/chainguard-dev/malcontent/pkg/malcontent"
1313
"github.com/chainguard-dev/malcontent/pkg/render"
1414
"github.com/chainguard-dev/malcontent/rules"
@@ -18,7 +18,7 @@ import (
1818

1919
func TestOCI(t *testing.T) {
2020
t.Parallel()
21-
ctx := slogtest.Context(t)
21+
ctx := context.Background()
2222
clog.FromContext(ctx).With("test", "scan_oci")
2323

2424
var out bytes.Buffer

pkg/action/scan.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,11 @@ func processArchive(ctx context.Context, c malcontent.Config, rfs []fs.FS, archi
630630

631631
tmpRoot, err := archive.ExtractArchiveToTempDir(ctx, archivePath)
632632
if err != nil {
633+
// Avoid failing an entire scan when encountering problematic archives
634+
// e.g., joblib_0.8.4_compressed_pickle_py27_np17.gz: not a valid gzip archive
635+
if !c.ExitExtraction {
636+
return nil, nil
637+
}
633638
return nil, fmt.Errorf("extract to temp: %w", err)
634639
}
635640
// Ensure that tmpRoot is removed before returning if created successfully
@@ -640,8 +645,8 @@ func processArchive(ctx context.Context, c malcontent.Config, rfs []fs.FS, archi
640645
}()
641646

642647
// macOS will prefix temporary directories with `/private`
643-
// update tmpRoot with this prefix to allow strings.TrimPrefix to work
644-
if runtime.GOOS == "darwin" {
648+
// update tmpRoot (if populated) with this prefix to allow strings.TrimPrefix to work
649+
if runtime.GOOS == "darwin" && tmpRoot != "" {
645650
tmpRoot = fmt.Sprintf("/private%s", tmpRoot)
646651
}
647652

pkg/action/testdata/17419.zip

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
Advisory :
2+
3+
4+
Abysssec Public Exploit :
5+
6+
This module exploits a code execution vulnerability in Mozilla
7+
Firefox <= 3.6.16 caused by nsTreeSelection element. The specific flaw
8+
exists within the way Firefox handles user defined functions of
9+
a nsTreeSelection element. When executing the function
10+
invalidateSelection it is possible to free the nsTreeSelection object
11+
that the function operates on. Any further operations on the freed
12+
object can result in remote code execution.this exploit module is only
13+
tested on win7 and used a Another JAVA ROPto defeat DEP/ASLR (due to
14+
there is no more non-aslr module in Firefox) and in my tests works
15+
reliably on Windows7.
16+
17+
there is two version of this exploit XP and 7 and both use different
18+
method that used in MSF Exploit bounty !
19+
20+
XP Version: https://gitlab.com/exploit-database/exploitdb-bin-sploits/-/raw/main/bin-sploits/17419-1.zip (nsTreeRange_XP.zip)
21+
Win7 Version: https://gitlab.com/exploit-database/exploitdb-bin-sploits/-/raw/main/bin-sploits/17419-2.zip (nsTreeRange_7.zip)
22+
23+
24+
25+
26+
questions / comments : Info [at] abysssec.com
Binary file not shown.

pkg/archive/archive.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ func extractNestedArchive(ctx context.Context, d string, f string, extracted *sy
6060
return fmt.Errorf("failed to determine file type: %w", err)
6161
}
6262

63+
// Empty filetypes or invalid MIME types should not be evaluated via `extract`
64+
if ft == nil || (ft != nil && ft.MIME == "") {
65+
return nil
66+
}
67+
6368
switch {
6469
case ft != nil && ft.MIME == "application/x-upx":
6570
isArchive = true
@@ -148,6 +153,11 @@ func ExtractArchiveToTempDir(ctx context.Context, path string) (string, error) {
148153
return "", fmt.Errorf("failed to determine file type: %w", err)
149154
}
150155

156+
// Empty filetypes or invalid MIME types should not be evaluated via `extract`
157+
if ft == nil || (ft != nil && ft.MIME == "") {
158+
return "", fmt.Errorf("unsupported archive type: %s", path)
159+
}
160+
151161
switch {
152162
case ft != nil && ft.MIME == "application/zlib":
153163
extract = ExtractZlib

pkg/archive/gzip.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,16 @@ import (
1313
gzip "github.com/klauspost/pgzip"
1414
)
1515

16+
var gzMIME = map[string]struct{}{
17+
"application/gzip": {},
18+
"application/gzip-compressed": {},
19+
"application/gzipped": {},
20+
"application/x-gunzip": {},
21+
"application/x-gzip": {},
22+
"application/x-gzip-compressed": {},
23+
"gzip/document": {},
24+
}
25+
1626
// extractGzip extracts .gz archives.
1727
func ExtractGzip(ctx context.Context, d string, f string) error {
1828
if ctx.Err() != nil {
@@ -22,13 +32,13 @@ func ExtractGzip(ctx context.Context, d string, f string) error {
2232
// Check whether the provided file is a valid gzip archive
2333
var isGzip bool
2434
if ft, err := programkind.File(f); err == nil && ft != nil {
25-
if ft.MIME == "application/gzip" {
35+
if _, ok := gzMIME[ft.MIME]; ok {
2636
isGzip = true
2737
}
2838
}
2939

3040
if !isGzip {
31-
return fmt.Errorf("not a valid gzip archive")
41+
return nil
3242
}
3343

3444
logger := clog.FromContext(ctx).With("dir", d, "file", f)

pkg/archive/zip.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,21 @@ import (
1212

1313
"github.com/chainguard-dev/clog"
1414
"github.com/chainguard-dev/malcontent/pkg/pool"
15+
"github.com/chainguard-dev/malcontent/pkg/programkind"
1516
zip "github.com/klauspost/compress/zip"
1617
"golang.org/x/sync/errgroup"
1718
)
1819

1920
var initZipPool sync.Once
2021

22+
var zipMIME = map[string]struct{}{
23+
"application/java-archive": {},
24+
"application/x-wheel+zip": {},
25+
"application/x-zip": {},
26+
"application/x-zip-compressed": {},
27+
"application/zip": {},
28+
}
29+
2130
// ExtractZip extracts .jar and .zip archives.
2231
func ExtractZip(ctx context.Context, d string, f string) error {
2332
if ctx.Err() != nil {
@@ -39,6 +48,17 @@ func ExtractZip(ctx context.Context, d string, f string) error {
3948
return nil
4049
}
4150

51+
var isZip bool
52+
if ft, err := programkind.File(f); err == nil && ft != nil {
53+
if _, ok := zipMIME[ft.MIME]; ok {
54+
isZip = true
55+
}
56+
}
57+
58+
if !isZip {
59+
return nil
60+
}
61+
4262
read, err := zip.OpenReader(f)
4363
if err != nil {
4464
return fmt.Errorf("failed to open zip file %s: %w", f, err)

pkg/malcontent/malcontent.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type Renderer interface {
2323

2424
type Config struct {
2525
Concurrency int
26+
ExitExtraction bool
2627
ExitFirstHit bool
2728
ExitFirstMiss bool
2829
FileRiskChange bool

0 commit comments

Comments
 (0)