Skip to content

Commit

Permalink
Extract IsInterestingExecutable from gobinary's FileRequired
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 713069569
  • Loading branch information
rpbeltran authored and copybara-github committed Jan 15, 2025
1 parent 9491c94 commit e6396f7
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 23 deletions.
54 changes: 54 additions & 0 deletions extractor/filesystem/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"
"path/filepath"
"regexp"
"runtime"
"strings"
"time"

Expand Down Expand Up @@ -545,3 +546,56 @@ func (i *ScanInput) GetRealPath() (string, error) {
io.Copy(f, i.Reader)
return path, nil
}

// TODO(b/380419487): This list is not exhaustive. We should add more extensions here.
var unlikelyExecutableExtensions = map[string]bool{
".c": true,
".cc": true,
".cargo-ok": true,
".crate": true,
".css": true,
".db": true,
".gitattributes": true,
".gitignore": true,
".go": true,
".h": true,
".html": true,
".json": true,
".lock": true,
".log": true,
".md": true,
".mod": true,
".png": true,
".proto": true,
".rs": true,
".stderr": true,
".sum": true,
".svg": true,
".tar": true,
".tmpl": true,
".toml": true,
".txt": true,
".woff2": true,
".xml": true,
".yaml": true,
".yml": true,
".zip": true,
".ziphash": true,
}

// IsInterestingExecutable returns true if the specified file is an executable which may need scanning.
func IsInterestingExecutable(api FileAPI) bool {
extension := filepath.Ext(api.Path())
if unlikelyExecutableExtensions[extension] {
return false
}

// TODO(b/279138598): Research: Maybe on windows all files have the executable bit set.
// Either windows .exe/.dll or unix executable bit should be set.
if runtime.GOOS == "windows" || extension == ".exe" || extension == ".dll" {
return true
}

mode, err := api.Stat()
return err == nil && mode.Mode()&0111 != 0
}
97 changes: 97 additions & 0 deletions extractor/filesystem/filesystem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -713,3 +714,99 @@ func TestRunFS_ReadError(t *testing.T) {
t.Errorf("extractor.Run(%v): unexpected status (-want +got):\n%s", ex, diff)
}
}

type fakeFileAPI struct {
path string
info fakefs.FakeFileInfo
}

func (f fakeFileAPI) Path() string { return f.path }
func (f fakeFileAPI) Stat() (fs.FileInfo, error) {
return f.info, nil
}

func TestIsInterestingExecutable(t *testing.T) {
tests := []struct {
name string
path string
mode fs.FileMode
want bool
wantWindows 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: "windows dll",
path: "some/path/a.dll",
mode: 0666,
want: true,
},
{
name: "not executable bit set",
path: "some/path/a",
mode: 0640,
want: false,
wantWindows: true,
},
{
name: "executable required",
path: "some/path/a",
mode: 0766,
want: true,
},
{
name: "unwanted extension",
path: "some/path/a.html",
mode: 0766,
want: false,
},
{
name: "another unwanted extension",
path: "some/path/a.txt",
mode: 0766,
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := filesystem.IsInterestingExecutable(fakeFileAPI{tt.path, fakefs.FakeFileInfo{
FileName: filepath.Base(tt.path),
FileMode: tt.mode,
}})

want := tt.want
// For Windows we don't check the executable bit on files.
if runtime.GOOS == "windows" && !want {
want = tt.wantWindows
}

if got != want {
t.Fatalf("FileRequired(%s): got %v, want %v", tt.path, got, want)
}

})

Check failure on line 810 in extractor/filesystem/filesystem_test.go

View workflow job for this annotation

GitHub Actions / lint

unnecessary trailing newline (whitespace)

Check failure on line 810 in extractor/filesystem/filesystem_test.go

View workflow job for this annotation

GitHub Actions / lint-just-new

unnecessary trailing newline (whitespace)
}
}
22 changes: 5 additions & 17 deletions extractor/filesystem/language/golang/gobinary/gobinary.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"errors"
"io"
"io/fs"
"path/filepath"
"runtime/debug"
"strings"

Expand Down Expand Up @@ -87,32 +86,21 @@ func (e Extractor) Requirements() *plugin.Capabilities { return &plugin.Capabili

// FileRequired returns true if the specified file is marked executable.
func (e Extractor) FileRequired(api filesystem.FileAPI) bool {
path := api.Path()

// TODO(b/380419487): This is inefficient, it would be better if gobinary would filter out common
// non executable by their file extension.
fileinfo, err := api.Stat()
if err != nil {
return false
}

if !fileinfo.Mode().IsRegular() {
// Includes dirs, symlinks, sockets, pipes...
if !filesystem.IsInterestingExecutable(api) {
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 {
fileinfo, err := api.Stat()
if err != nil {
return false
}

if e.maxFileSizeBytes > 0 && fileinfo.Size() > e.maxFileSizeBytes {
e.reportFileRequired(path, fileinfo.Size(), stats.FileRequiredResultSizeLimitExceeded)
e.reportFileRequired(api.Path(), fileinfo.Size(), stats.FileRequiredResultSizeLimitExceeded)
return false
}

e.reportFileRequired(path, fileinfo.Size(), stats.FileRequiredResultOK)
e.reportFileRequired(api.Path(), fileinfo.Size(), stats.FileRequiredResultOK)
return true
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,6 @@ func TestFileRequired(t *testing.T) {
mode: 0640,
wantRequired: false,
},
{
name: "Non regular file, socket",
path: "some/path/a",
mode: fs.ModeSocket | 0777,
wantRequired: false,
},
{
name: "executable required if size less than maxFileSizeBytes",
path: "some/path/a",
Expand Down

0 comments on commit e6396f7

Please sign in to comment.