diff --git a/README.md b/README.md index b151d7f..9d705b5 100644 --- a/README.md +++ b/README.md @@ -173,6 +173,7 @@ directory you can supply `./...` as the input argument. - G114: Use of net/http serve function that has no support for setting timeouts - G115: Potential integer overflow when converting between integer types - G116: Detect Trojan Source attacks using bidirectional Unicode control characters +- G117: Potential exposure of secrets via JSON marshaling - G201: SQL query construction using format string - G202: SQL query construction using string concatenation - G203: Use of unescaped data in HTML templates diff --git a/autofix/openai.go b/autofix/openai.go index 994ecdd..b40d064 100644 --- a/autofix/openai.go +++ b/autofix/openai.go @@ -21,7 +21,7 @@ var _ GenAIClient = (*openaiWrapper)(nil) type OpenAIConfig struct { Model string - APIKey string + APIKey string `json:"-"` BaseURL string MaxTokens int Temperature float64 diff --git a/cwe/data.go b/cwe/data.go index a9568ba..ee95294 100644 --- a/cwe/data.go +++ b/cwe/data.go @@ -118,6 +118,11 @@ var idWeaknesses = map[string]*Weakness{ Description: "The software does not handle or incorrectly handles a compressed input with a very high compression ratio that produces a large output.", Name: "Improper Handling of Highly Compressed Data (Data Amplification)", }, + "499": { + ID: "499", + Description: "The code contains a class with sensitive data, but the class does not explicitly deny serialization. The data can be accessed by serializing the class through another class.", + Name: "Serializable Class Containing Sensitive Data", + }, "676": { ID: "676", Description: "The program invokes a potentially dangerous function that could introduce a vulnerability if it is used incorrectly, but the function can also be used safely.", diff --git a/issue/issue.go b/issue/issue.go index 28c876b..3648ace 100644 --- a/issue/issue.go +++ b/issue/issue.go @@ -68,6 +68,7 @@ var ruleToCWE = map[string]string{ "G114": "676", "G115": "190", "G116": "838", + "G117": "499", "G201": "89", "G202": "89", "G203": "79", diff --git a/rules/rulelist.go b/rules/rulelist.go index bd8dbbb..7362f2e 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -77,6 +77,7 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList { {"G112", "Detect ReadHeaderTimeout not configured as a potential risk", NewSlowloris}, {"G114", "Use of net/http serve function that has no support for setting timeouts", NewHTTPServeWithoutTimeouts}, {"G116", "Detect Trojan Source attacks using bidirectional Unicode characters", NewTrojanSource}, + {"G117", "Potential exposure of secrets via JSON marshaling", NewSecretSerialization}, // injection {"G201", "SQL query construction using format string", NewSQLStrFormat}, diff --git a/rules/rules_test.go b/rules/rules_test.go index 113ffb4..fc5d1bb 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -111,6 +111,10 @@ var _ = Describe("gosec rules", func() { runner("G116", testutils.SampleCodeG116) }) + It("should detect exported struct fields that may contain secrets and are JSON serializable", func() { + runner("G117", testutils.SampleCodeG117) + }) + It("should detect sql injection via format strings", func() { runner("G201", testutils.SampleCodeG201) }) diff --git a/rules/secret_serialization.go b/rules/secret_serialization.go new file mode 100644 index 0000000..8dc674a --- /dev/null +++ b/rules/secret_serialization.go @@ -0,0 +1,123 @@ +package rules + +import ( + "fmt" + "go/ast" + "reflect" + "regexp" + "strconv" + "strings" + + "github.com/securego/gosec/v2" + "github.com/securego/gosec/v2/issue" +) + +type secretSerialization struct { + issue.MetaData + pattern *regexp.Regexp +} + +func (r *secretSerialization) ID() string { + return r.MetaData.ID +} + +func (r *secretSerialization) Match(n ast.Node, ctx *gosec.Context) (*issue.Issue, error) { + field, ok := n.(*ast.Field) + if !ok || len(field.Names) == 0 { + return nil, nil // skip embedded (anonymous) fields + } + + // Parse the JSON tag to determine behavior + omitted := false + jsonKey := "" + + if field.Tag != nil { + if tagVal, err := strconv.Unquote(field.Tag.Value); err == nil && tagVal != "" { + st := reflect.StructTag(tagVal) + if tag := st.Get("json"); tag != "" { + if tag == "-" { + omitted = true + } else { + // "name,omitempty" -> "name" + // "-," -> "-" (A field literally named "-") + parts := strings.SplitN(tag, ",", 2) + jsonKey = parts[0] + } + } + } + } + + if omitted { + return nil, nil + } + + // Iterate over all names in this field definition + // e.g., type T struct { Pass, Salt string } + isSensitiveType := false + switch t := field.Type.(type) { + case *ast.Ident: + if t.Name == "string" { + isSensitiveType = true + } + case *ast.StarExpr: + if ident, ok := t.X.(*ast.Ident); ok && ident.Name == "string" { + isSensitiveType = true + } + case *ast.ArrayType: + if star, ok := t.Elt.(*ast.StarExpr); ok { + if ident, ok := star.X.(*ast.Ident); ok && ident.Name == "string" { + isSensitiveType = true // []*string + } + } else if ident, ok := t.Elt.(*ast.Ident); ok { + if ident.Name == "string" || ident.Name == "byte" { + isSensitiveType = true // []string or []byte + } + } + } + + if !isSensitiveType { + return nil, nil + } + + // Check each named exported field + for _, nameIdent := range field.Names { + fieldName := nameIdent.Name + if fieldName == "_" || !ast.IsExported(fieldName) { + continue + } + + effectiveKey := jsonKey + if effectiveKey == "" { + effectiveKey = fieldName + } + + if r.pattern.MatchString(fieldName) || r.pattern.MatchString(effectiveKey) { + msg := fmt.Sprintf("Exported struct field %q (JSON key %q) matches secret pattern", fieldName, effectiveKey) + return ctx.NewIssue(field, r.ID(), msg, r.Severity, r.Confidence), nil + } + } + + return nil, nil +} + +func NewSecretSerialization(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { + patternStr := `(?i)\b((?:api|access|auth|bearer|client|oauth|private|refresh|session|jwt)[_-]?(?:key|secret|token)s?|password|passwd|pwd|pass|secret|cred|jwt)\b` + + if val, ok := conf[id]; ok { + if m, ok := val.(map[string]interface{}); ok { + if p, ok := m["pattern"].(string); ok && p != "" { + patternStr = p + } + } + } + + return &secretSerialization{ + pattern: regexp.MustCompile(patternStr), + MetaData: issue.MetaData{ + ID: id, + What: "Exported struct field appears to be a secret and is not ignored by JSON marshaling", + Severity: issue.Medium, + Confidence: issue.Medium, + }, + }, []ast.Node{(*ast.Field)(nil)} +} diff --git a/testutils/g117_samples.go b/testutils/g117_samples.go new file mode 100644 index 0000000..ba1bd77 --- /dev/null +++ b/testutils/g117_samples.go @@ -0,0 +1,197 @@ +// testutils/g117_samples.go +package testutils + +import "github.com/securego/gosec/v2" + +var SampleCodeG117 = []CodeSample{ + // Positive: match on field name (default JSON key) + {[]string{` +package main + +type Config struct { + Password string +} +`}, 1, gosec.NewConfig()}, + {[]string{` +package main + +type Config struct { + APIKey *string ` + "`json:\"api_key\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + {[]string{` +package main + +type Config struct { + PrivateKey []byte ` + "`json:\"private_key\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + // Positive: match on field name (explicit non-sensitive JSON key) + {[]string{` +package main + +type Config struct { + Password string ` + "`json:\"text_field\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + // Positive: match on JSON key (non-sensitive field name) + {[]string{` +package main + +type Config struct { + SafeField string ` + "`json:\"api_key\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + // Positive: match on both + {[]string{` +package main + +type Config struct { + Token string ` + "`json:\"auth_token\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + // Positive: snake/hyphen variants in JSON key + {[]string{` +package main + +type Config struct { + Key string ` + "`json:\"access-key\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + // Positive: empty json tag part falls back to field name + {[]string{` +package main + +type Config struct { + Secret string ` + "`json:\",omitempty\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + // Positive: plural forms + {[]string{` +package main + +type Config struct { + ApiTokens []string +} +`}, 1, gosec.NewConfig()}, + + {[]string{` +package main + +type Config struct { + RefreshTokens []string ` + "`json:\"refresh_tokens\"`" + ` +} +`}, 1, gosec.NewConfig()}, + + {[]string{` +package main + +type Config struct { + AccessTokens []*string +} +`}, 1, gosec.NewConfig()}, + + {[]string{` +package main + +type Config struct { + CustomSecret string ` + "`json:\"my_custom_secret\"`" + ` +} +`}, 1, func() gosec.Config { + cfg := gosec.NewConfig() + cfg.Set("G117", map[string]interface{}{ + "pattern": "(?i)custom[_-]?secret", + }) + return cfg + }()}, + + // Negative: json:"-" (omitted) + {[]string{` +package main + +type Config struct { + Password string ` + "`json:\"-\"`" + ` +} +`}, 0, gosec.NewConfig()}, + + // Negative: both field name and JSON key non-sensitive + {[]string{` +package main + +type Config struct { + UserID string ` + "`json:\"user_id\"`" + ` +} +`}, 0, gosec.NewConfig()}, + + // Negative: unexported field + {[]string{` +package main + +type Config struct { + password string +} +`}, 0, gosec.NewConfig()}, + + // Negative: non-sensitive type (int) even with "token" + {[]string{` +package main + +type Config struct { + MaxTokens int +} +`}, 0, gosec.NewConfig()}, + + // Negative: non-secret plural slice (common FP like redaction placeholders) + {[]string{` +package main + +type Config struct { + RedactionTokens []string ` + "`json:\"redactionTokens,omitempty\"`" + ` +} +`}, 0, gosec.NewConfig()}, + + // Negative: grouped fields, only one sensitive (should still flag the sensitive one) + // Note: we expect 1 issue (for the sensitive field) + {[]string{` +package main + +type Config struct { + Safe, Password string +} +`}, 1, gosec.NewConfig()}, + + // Suppression: trailing line comment + {[]string{` +package main + +type Config struct { + Password string // #nosec G117 +} +`}, 0, gosec.NewConfig()}, + + // Suppression: line comment above field + {[]string{` +package main + +type Config struct { + // #nosec G117 -- false positive + Password string +} +`}, 0, gosec.NewConfig()}, + + // Suppression: trailing with justification + {[]string{` +package main + +type Config struct { + APIKey string ` + "`json:\"api_key\"`" + ` // #nosec G117 -- public key +} +`}, 0, gosec.NewConfig()}, +}