From 2f67eaad49176543613e73d7647f94c9ccfa0522 Mon Sep 17 00:00:00 2001 From: BiswajeetRay7 Date: Mon, 13 Jul 2026 15:11:48 -0400 Subject: [PATCH] fix(security): sanitize file paths in log messages to prevent GitHub Actions workflow command injection Several log sites echo file paths derived from filesystem walk of the scanned repository without applying output.SanitizeForWorkflowCommand. On Linux file names may contain \r/\n bytes and git tracks such files, so an attacker-controlled repository (typically via pull_request-triggered osv-scanner-action) could inject ::add-mask::, ::stop-commands::, ::error::, etc. into the runner's stdout. pkg/osvscanner/scan.go:158 already applies the correct guard for the directory-name log line and its source comment explains the threat model. This change extends the same guard to the remaining unsanitized sites: - internal/config/manager.go:57,61 (config LoadPath / configPath) - internal/config/config.go:164 (LoadPath on duplicate-ignores warning) - pkg/osvscanner/osvscanner.go:401 (configFile on unused-ignores warning) - internal/sourceanalysis/rust.go:55,62,73 (rust binary path in analysis errors) Refs: https://issuetracker.google.com/issues/533784193 --- internal/config/config.go | 3 ++- internal/config/manager.go | 5 +++-- internal/sourceanalysis/rust.go | 7 ++++--- pkg/osvscanner/osvscanner.go | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index c70e7068940..7b297c31ea2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,6 +9,7 @@ import ( "github.com/google/osv-scanner/v2/internal/cachedregexp" "github.com/google/osv-scanner/v2/internal/cmdlogger" "github.com/google/osv-scanner/v2/internal/imodels" + "github.com/google/osv-scanner/v2/internal/output" ) var OSVScannerConfigName = "osv-scanner.toml" @@ -161,7 +162,7 @@ func (c *Config) warnAboutDuplicates() { for _, vuln := range c.IgnoredVulns { if _, ok := seen[vuln.ID]; ok { - cmdlogger.Warnf("warning: %s has multiple ignores for %s - only the first will be used!", c.LoadPath, vuln.ID) + cmdlogger.Warnf("warning: %s has multiple ignores for %s - only the first will be used!", output.SanitizeForWorkflowCommand(c.LoadPath), vuln.ID) } seen[vuln.ID] = struct{}{} } diff --git a/internal/config/manager.go b/internal/config/manager.go index 4294ebcf4f9..f7a8d41ce23 100644 --- a/internal/config/manager.go +++ b/internal/config/manager.go @@ -10,6 +10,7 @@ import ( "github.com/BurntSushi/toml" "github.com/google/osv-scanner/v2/internal/cachedregexp" "github.com/google/osv-scanner/v2/internal/cmdlogger" + "github.com/google/osv-scanner/v2/internal/output" ) type Manager struct { @@ -54,11 +55,11 @@ func (m *Manager) Get(targetPath string) Config { config, configErr := tryLoadConfig(configPath) if configErr == nil { - cmdlogger.Infof("Loaded filter from: %s", config.LoadPath) + cmdlogger.Infof("Loaded filter from: %s", output.SanitizeForWorkflowCommand(config.LoadPath)) } else { // anything other than the config file not existing is most likely due to an invalid config file if !errors.Is(configErr, os.ErrNotExist) { - cmdlogger.Errorf("Ignored invalid config file at %s because: %v", configPath, configErr) + cmdlogger.Errorf("Ignored invalid config file at %s because: %v", output.SanitizeForWorkflowCommand(configPath), configErr) } // If config doesn't exist, use the default config config = m.DefaultConfig diff --git a/internal/sourceanalysis/rust.go b/internal/sourceanalysis/rust.go index b6a1c8720db..396bf5f7433 100644 --- a/internal/sourceanalysis/rust.go +++ b/internal/sourceanalysis/rust.go @@ -19,6 +19,7 @@ import ( "github.com/google/osv-scanner/v2/internal/thirdparty/ar" "github.com/google/osv-scanner/v2/pkg/models" "github.com/ianlancetaylor/demangle" + "github.com/google/osv-scanner/v2/internal/output" ) const ( @@ -52,14 +53,14 @@ func rustAnalysis(pkgs []models.PackageVulns, source models.SourceInfo) { // Is a library, so need an extra step to extract the object binary file before passing to parseDWARFData buf, err := extractRlibArchive(path) if err != nil { - cmdlogger.Errorf("failed to analyse '%s': %s", path, err) + cmdlogger.Errorf("failed to analyse '%s': %s", output.SanitizeForWorkflowCommand(path), err) continue } readAt = bytes.NewReader(buf.Bytes()) } else { f, err := os.Open(path) if err != nil { - cmdlogger.Errorf("failed to read binary '%s': %s", path, err) + cmdlogger.Errorf("failed to read binary '%s': %s", output.SanitizeForWorkflowCommand(path), err) continue } // This is fine to defer til the end of the function as there's @@ -70,7 +71,7 @@ func rustAnalysis(pkgs []models.PackageVulns, source models.SourceInfo) { calls, err := functionsFromDWARF(readAt) if err != nil { - cmdlogger.Errorf("failed to analyse '%s': %s", path, err) + cmdlogger.Errorf("failed to analyse '%s': %s", output.SanitizeForWorkflowCommand(path), err) continue } diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go index e3e86a81efc..572ae7fa847 100644 --- a/pkg/osvscanner/osvscanner.go +++ b/pkg/osvscanner/osvscanner.go @@ -398,7 +398,7 @@ func finalizeScanResult(scanResult results.ScanResults, actions ScannerActions) slices.Sort(configFiles) for _, configFile := range configFiles { - cmdlogger.Warnf("%s has unused ignores:", configFile) + cmdlogger.Warnf("%s has unused ignores:", output.SanitizeForWorkflowCommand(configFile)) for _, iv := range unusedIgnoredEntries[configFile] { cmdlogger.Warnf(" - %s", iv.ID)