From c73f1272d581b155336e56b37c9ed73e9627e8d9 Mon Sep 17 00:00:00 2001 From: Ryan Beltran Date: Tue, 7 Jan 2025 15:56:26 -0800 Subject: [PATCH] Extract IsInterestingExecutable from gobinary's FileRequired PiperOrigin-RevId: 713069569 --- extractor/filesystem/filesystem.go | 19 ++++++ extractor/filesystem/filesystem_test.go | 64 +++++++++++++++++++ .../language/golang/gobinary/gobinary.go | 10 +-- 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/extractor/filesystem/filesystem.go b/extractor/filesystem/filesystem.go index dc07ece4..bae094dd 100644 --- a/extractor/filesystem/filesystem.go +++ b/extractor/filesystem/filesystem.go @@ -538,3 +538,22 @@ func (i *ScanInput) GetRealPath() (string, error) { io.Copy(f, i.Reader) return path, nil } + +// IsInterestingExecutable returns true if the specified file is an executable which may need scanning. +func IsInterestingExecutable(path string, fileInfo fs.FileInfo) bool { + // TODO(b/380419487): This is inefficient, it would be better to filter out common + // non executables by their file extension. + + // Ignore non regular files such as dirs, symlinks, sockets, pipes, etc.... + if !fileInfo.Mode().IsRegular() { + return false + } + + // TODO(b/279138598): Research: Maybe on windows all files have the executable bit set. + // Either windows .exe or unix executable bit should be set. + if filepath.Ext(path) != ".exe" && fileInfo.Mode()&0111 == 0 { + return false + } + + return true +} diff --git a/extractor/filesystem/filesystem_test.go b/extractor/filesystem/filesystem_test.go index aece0201..20ec4bed 100644 --- a/extractor/filesystem/filesystem_test.go +++ b/extractor/filesystem/filesystem_test.go @@ -39,6 +39,7 @@ import ( "github.com/google/osv-scalibr/plugin" "github.com/google/osv-scalibr/stats" fe "github.com/google/osv-scalibr/testing/fakeextractor" + "github.com/google/osv-scalibr/testing/fakefs" ) // pathsMapFS provides a hooked version of MapFS that forces slashes. Because depending on the @@ -713,3 +714,66 @@ func TestRunFS_ReadError(t *testing.T) { t.Errorf("extractor.Run(%v): unexpected status (-want +got):\n%s", ex, diff) } } + +func TestIsInterestingExecutable(t *testing.T) { + tests := []struct { + name string + path string + mode fs.FileMode + want bool + }{ + { + name: "user executable", + path: "some/path/a", + mode: 0766, + want: true, + }, + { + name: "group executable", + path: "some/path/a", + mode: 0676, + want: true, + }, + { + name: "other executable", + path: "some/path/a", + mode: 0667, + want: true, + }, + { + name: "windows exe", + path: "some/path/a.exe", + mode: 0666, + want: true, + }, + { + name: "not executable bit set", + path: "some/path/a", + mode: 0640, + want: false, + }, + { + name: "Non regular file, socket", + path: "some/path/a", + mode: fs.ModeSocket | 0777, + want: false, + }, + { + name: "executable required", + path: "some/path/a", + mode: 0766, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := filesystem.IsInterestingExecutable(tt.path, fakefs.FakeFileInfo{ + FileName: filepath.Base(tt.path), + FileMode: tt.mode, + }); got != tt.want { + t.Fatalf("FileRequired(%s): got %v, want %v", tt.path, got, tt.want) + } + }) + } +} diff --git a/extractor/filesystem/language/golang/gobinary/gobinary.go b/extractor/filesystem/language/golang/gobinary/gobinary.go index 63f9f91a..16ec61b0 100644 --- a/extractor/filesystem/language/golang/gobinary/gobinary.go +++ b/extractor/filesystem/language/golang/gobinary/gobinary.go @@ -22,7 +22,6 @@ import ( "errors" "io" "io/fs" - "path/filepath" "runtime/debug" "strings" @@ -96,14 +95,7 @@ func (e Extractor) FileRequired(api filesystem.FileAPI) bool { return false } - if !fileinfo.Mode().IsRegular() { - // Includes dirs, symlinks, sockets, pipes... - return false - } - - // TODO(b/279138598): Research: Maybe on windows all files have the executable bit set. - // Either windows .exe or unix executable bit should be set. - if filepath.Ext(path) != ".exe" && fileinfo.Mode()&0111 == 0 { + if !filesystem.IsInterestingExecutable(path, fileinfo) { return false }