diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8262c6..2419f17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,7 +38,7 @@ jobs: - name: Run Gosec Security Scanner uses: securego/gosec@master with: - args: ./... + args: '-exclude-dir=testdata ./...' - name: Run Tests run: make test - name: Perf Diff diff --git a/.github/workflows/scan.yml b/.github/workflows/scan.yml index a0204fc..0f55d76 100644 --- a/.github/workflows/scan.yml +++ b/.github/workflows/scan.yml @@ -18,7 +18,7 @@ jobs: uses: securego/gosec@master with: # we let the report trigger content trigger a failure using the GitHub Security features. - args: '-no-fail -fmt sarif -out results.sarif ./...' + args: '-no-fail -fmt sarif -out results.sarif -exclude-dir=testdata ./...' - name: Upload SARIF file uses: github/codeql-action/upload-sarif@v4 with: diff --git a/Makefile b/Makefile index d9880bb..12d70e2 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ golangci: sec: @echo "SECURITY SCANNING" - ./$(BIN) ./... + ./$(BIN) -exclude-dir=testdata ./... govulncheck: install-govulncheck @echo "CHECKING VULNERABILITIES" diff --git a/README.md b/README.md index 9d705b5..e5b07bb 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,21 @@ jobs: sarif_file: results.sarif ``` +### Go Analysis + +The `goanalysis` package provides a [`golang.org/x/tools/go/analysis.Analyzer`](https://pkg.go.dev/golang.org/x/tools/go/analysis) for integration with tools that support the standard Go analysis interface, such as Bazel's [nogo](https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst) framework: + +```starlark +nogo( + name = "nogo", + deps = [ + "@com_github_securego_gosec_v2//goanalysis", + # add more analyzers as needed + ], + visibility = ["//visibility:public"], +) +``` + ### Local Installation ```bash diff --git a/analyzer.go b/analyzer.go index 0508bce..3bd4d05 100644 --- a/analyzer.go +++ b/analyzer.go @@ -428,6 +428,24 @@ func (gosec *Analyzer) CheckAnalyzers(pkg *packages.Package) { return } + gosec.CheckAnalyzersWithSSA(pkg, ssaResult) +} + +// CheckAnalyzersWithSSA runs analyzers on a given package using a pre-built SSA result. +// This is useful when SSA has already been built by an external analysis framework +// (e.g., nogo) and you want to avoid rebuilding it, which can be expensive for large +// codebases and breaks caching strategies. +func (gosec *Analyzer) CheckAnalyzersWithSSA(pkg *packages.Package, ssaResult *buildssa.SSA) { + // significant performance improvement if no analyzers are loaded + if len(gosec.analyzerSet.Analyzers) == 0 { + return + } + + if ssaResult == nil { + gosec.logger.Print("CheckAnalyzersWithSSA called with nil SSA result") + return + } + resultMap := map[*analysis.Analyzer]interface{}{ buildssa.Analyzer: &analyzers.SSAAnalyzerResult{ Config: gosec.Config(), diff --git a/goanalysis/analyzer.go b/goanalysis/analyzer.go new file mode 100644 index 0000000..ce96673 --- /dev/null +++ b/goanalysis/analyzer.go @@ -0,0 +1,243 @@ +// (c) Copyright gosec's authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package goanalysis provides a standard golang.org/x/tools/go/analysis.Analyzer for gosec. +package goanalysis + +import ( + "fmt" + "go/token" + "io" + "log" + "strconv" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/buildssa" + "golang.org/x/tools/go/packages" + + "github.com/securego/gosec/v2" + "github.com/securego/gosec/v2/analyzers" + "github.com/securego/gosec/v2/issue" + "github.com/securego/gosec/v2/rules" +) + +const Doc = `gosec is a static analysis tool that scans Go code for security problems.` + +// Analyzer is the standard go/analysis Analyzer for gosec. +var Analyzer = &analysis.Analyzer{ + Name: "gosec", + Doc: Doc, + Run: run, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, +} + +var ( + flagIncludeRules string + flagExcludeRules string + flagExcludeGenerated bool + flagMinSeverity string + flagMinConfidence string +) + +//nolint:gochecknoinits // Required for go/analysis Analyzer flag registration +func init() { + Analyzer.Flags.StringVar(&flagIncludeRules, "include", "", "Comma-separated list of rule IDs to include (e.g., G101,G102)") + Analyzer.Flags.StringVar(&flagExcludeRules, "exclude", "", "Comma-separated list of rule IDs to exclude (e.g., G104)") + Analyzer.Flags.BoolVar(&flagExcludeGenerated, "exclude-generated", true, "Exclude generated code from analysis") + Analyzer.Flags.StringVar(&flagMinSeverity, "severity", "low", "Minimum severity: low, medium, or high") + Analyzer.Flags.StringVar(&flagMinConfidence, "confidence", "low", "Minimum confidence: low, medium, or high") +} + +func run(pass *analysis.Pass) (any, error) { + // Create gosec config and analyzer + config := gosec.NewConfig() + logger := log.New(io.Discard, "", 0) // Discard gosec's verbose logging + gosecAnalyzer := gosec.NewAnalyzer(config, false, flagExcludeGenerated, false, 1, logger) + + // Build filters from include/exclude flags + ruleFilters := buildFilters(flagIncludeRules, flagExcludeRules, rules.NewRuleFilter) + analyzerFilters := buildFilters(flagIncludeRules, flagExcludeRules, analyzers.NewAnalyzerFilter) + + // Load rules and analyzers + ruleList := rules.Generate(false, ruleFilters...) + ruleBuilders, ruleSuppressed := ruleList.RulesInfo() + gosecAnalyzer.LoadRules(ruleBuilders, ruleSuppressed) + + analyzerList := analyzers.Generate(false, analyzerFilters...) + analyzerDefs, analyzerSuppressed := analyzerList.AnalyzersInfo() + gosecAnalyzer.LoadAnalyzers(analyzerDefs, analyzerSuppressed) + + // Convert analysis.Pass to packages.Package + pkg := convertPassToPackage(pass) + + // Run gosec AST-based rules on the package + // This populates context.Ignores with nosec suppressions from comments + gosecAnalyzer.CheckRules(pkg) + + // Run SSA-based analyzers using the cached SSA result provided by the analysis framework + // This reuses the SSA already built, maintaining cache efficiency + // Both AST and SSA issues will respect nosec comments via gosec's updateIssues() + if ssaResult, ok := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA); ok && ssaResult != nil { + gosecAnalyzer.CheckAnalyzersWithSSA(pkg, ssaResult) + } + + // Get all results (both AST and SSA, with nosec filtering already applied) + issues, _, _ := gosecAnalyzer.Report() + + // Report issues as diagnostics, filtering by severity and confidence + minSev, err := parseScore(flagMinSeverity) + if err != nil { + return nil, fmt.Errorf("invalid severity %q: %w", flagMinSeverity, err) + } + minConf, err := parseScore(flagMinConfidence) + if err != nil { + return nil, fmt.Errorf("invalid confidence %q: %w", flagMinConfidence, err) + } + + for _, iss := range issues { + if iss.Severity < minSev || iss.Confidence < minConf { + continue + } + + pos := parsePosition(pass.Fset, iss) + what := iss.What + if iss.Cwe != nil && iss.Cwe.ID != "" { + what = fmt.Sprintf("[%s] %s", iss.Cwe.SprintID(), iss.What) + } + msg := fmt.Sprintf("%s: %s (Severity: %s, Confidence: %s)", iss.RuleID, what, iss.Severity.String(), iss.Confidence.String()) + + // If we can't locate the issue, report it anyway but note the location problem + if pos == token.NoPos { + msg = fmt.Sprintf("%s [unable to locate %s:%s]", msg, iss.File, iss.Line) + } + + pass.Report(analysis.Diagnostic{ + Pos: pos, + Category: iss.RuleID, + Message: msg, + }) + } + + return nil, nil +} + +// convertPassToPackage converts an analysis.Pass to a packages.Package +// that gosec expects. This allows us to reuse gosec's existing analysis logic. +func convertPassToPackage(pass *analysis.Pass) *packages.Package { + pkg := &packages.Package{ + Name: pass.Pkg.Name(), + Fset: pass.Fset, + Syntax: pass.Files, + Types: pass.Pkg, + TypesInfo: pass.TypesInfo, + TypesSizes: pass.TypesSizes, + } + + // Populate file names for the package + pkg.CompiledGoFiles = make([]string, len(pass.Files)) + for i, f := range pass.Files { + pkg.CompiledGoFiles[i] = pass.Fset.File(f.Pos()).Name() + } + + return pkg +} + +// buildFilters creates include/exclude filters from comma-separated rule IDs +func buildFilters[T any](include, exclude string, newFilter func(bool, ...string) T) []T { + var filters []T + if include != "" { + if ids := parseRuleIDs(include); len(ids) > 0 { + filters = append(filters, newFilter(false, ids...)) + } + } + if exclude != "" { + if ids := parseRuleIDs(exclude); len(ids) > 0 { + filters = append(filters, newFilter(true, ids...)) + } + } + return filters +} + +// parseRuleIDs parses a comma-separated list of rule IDs +func parseRuleIDs(s string) []string { + parts := strings.Split(s, ",") + ids := make([]string, 0, len(parts)) + for _, p := range parts { + if id := strings.TrimSpace(p); id != "" { + ids = append(ids, id) + } + } + return ids +} + +// parseScore converts a severity/confidence string to issue.Score +func parseScore(s string) (issue.Score, error) { + switch strings.ToLower(s) { + case "high": + return issue.High, nil + case "medium": + return issue.Medium, nil + case "low": + return issue.Low, nil + default: + return issue.Low, fmt.Errorf("must be low, medium, or high") + } +} + +// parsePosition converts a gosec issue location to a token.Pos +func parsePosition(fset *token.FileSet, iss *issue.Issue) token.Pos { + var file *token.File + fset.Iterate(func(f *token.File) bool { + if f.Name() == iss.File { + file = f + return false + } + return true + }) + + if file == nil { + return token.NoPos + } + + // Handle line ranges (e.g., "28-34") by using the start line + lineStr := iss.Line + if idx := strings.Index(lineStr, "-"); idx > 0 { + lineStr = lineStr[:idx] + } + + line, err := strconv.Atoi(lineStr) + if err != nil || line < 1 || line > file.LineCount() { + return token.NoPos + } + + lineStart := file.LineStart(line) + + // Add column offset if available (column is 1-based) + col, err := strconv.Atoi(iss.Col) + if err != nil || col < 1 { + return lineStart + } + + // Calculate position: lineStart is the position of the first character, + // so we add (col - 1) to get the correct column position + pos := lineStart + token.Pos(col-1) + + // Ensure we don't exceed file bounds + if int(pos) > file.Base()+file.Size() { + return lineStart + } + + return pos +} diff --git a/goanalysis/analyzer_test.go b/goanalysis/analyzer_test.go new file mode 100644 index 0000000..46c52d7 --- /dev/null +++ b/goanalysis/analyzer_test.go @@ -0,0 +1,27 @@ +// (c) Copyright gosec's authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package goanalysis_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/securego/gosec/v2/goanalysis" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), goanalysis.Analyzer, "a") +} diff --git a/goanalysis/testdata/src/a/basic_output.go b/goanalysis/testdata/src/a/basic_output.go new file mode 100644 index 0000000..d3abd08 --- /dev/null +++ b/goanalysis/testdata/src/a/basic_output.go @@ -0,0 +1,43 @@ +package a + +import ( + "crypto/md5" // want `G501: \[CWE-327\] Blocklisted import crypto/md5: weak cryptographic primitive` + "fmt" + "os/exec" +) + +func VulnerableFunction() { + // Test SQL injection - gosec doesn't catch simple string concatenation without database/sql + query := "SELECT * FROM users WHERE name = '" + getUserInput() + "'" + _ = query + + // G204: Command injection (AST-based rule) + cmd := exec.Command("sh", "-c", getUserInput()) // want `G204: \[CWE-78\] Subprocess launched with a potential tainted input or cmd arguments` + _ = cmd + + // G401: Weak crypto (AST-based rule) + h := md5.New() // want `G401: \[CWE-328\] Use of weak cryptographic primitive` + _ = h +} + +func getUserInput() string { + return "test" +} + +func SecureFunction() { + fmt.Println("This is secure") +} + +func IntegerOverflow() { + // G115: Integer overflow in type conversion (SSA-based analyzer) + var a uint32 = 0xFFFFFFFF + b := int32(a) // want `G115` + fmt.Println(b) +} + +func SliceBounds() { + // G602: Slice bounds check (SSA-based analyzer) + s := []int{1, 2, 3} + idx := 10 + _ = s[:idx] // want `G602` +} diff --git a/goanalysis/testdata/src/a/nosec.go b/goanalysis/testdata/src/a/nosec.go new file mode 100644 index 0000000..c500d2c --- /dev/null +++ b/goanalysis/testdata/src/a/nosec.go @@ -0,0 +1,37 @@ +package a + +import ( + "crypto/md5" // want `G501` + "crypto/sha1" // want `G505` + "os/exec" +) + +func NosecVariants() { + // Basic #nosec comment suppresses the diagnostic + h1 := md5.New() // #nosec + _ = h1 + + // #nosec with rule ID + h2 := md5.New() // #nosec G401 + _ = h2 + + // #nosec with multiple rule IDs + h3 := sha1.New() // #nosec G401 G505 + _ = h3 + + // nosec without # should NOT suppress + h4 := md5.New() // nosec // want `G401` + _ = h4 + + // Wrong rule ID should NOT suppress (G204 != G401) + h5 := md5.New() // #nosec G204 -- wrong rule // want `G401` + _ = h5 + + // #nosec with explanation + h6 := md5.New() // #nosec G401 -- used for non-cryptographic checksum + _ = h6 + + // Command injection with #nosec + cmd := exec.Command("sh", "-c", getUserInput()) // #nosec G204 + _ = cmd +}