Skip to content

Commit 0f6f21c

Browse files
oittaaoittaa
authored
feat: add secrets serialization G117 (#1451)
* Rule to detect secrets serialization * Add G117 to rules_test.go * Fix false positives * Map to CWE 499, update README
1 parent 717706e commit 0f6f21c

8 files changed

Lines changed: 333 additions & 1 deletion

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ directory you can supply `./...` as the input argument.
173173
- G114: Use of net/http serve function that has no support for setting timeouts
174174
- G115: Potential integer overflow when converting between integer types
175175
- G116: Detect Trojan Source attacks using bidirectional Unicode control characters
176+
- G117: Potential exposure of secrets via JSON marshaling
176177
- G201: SQL query construction using format string
177178
- G202: SQL query construction using string concatenation
178179
- G203: Use of unescaped data in HTML templates

‎autofix/openai.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ var _ GenAIClient = (*openaiWrapper)(nil)
2121

2222
type OpenAIConfig struct {
2323
Model string
24-
APIKey string
24+
APIKey string `json:"-"`
2525
BaseURL string
2626
MaxTokens int
2727
Temperature float64

‎cwe/data.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ var idWeaknesses = map[string]*Weakness{
118118
Description: "The software does not handle or incorrectly handles a compressed input with a very high compression ratio that produces a large output.",
119119
Name: "Improper Handling of Highly Compressed Data (Data Amplification)",
120120
},
121+
"499": {
122+
ID: "499",
123+
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.",
124+
Name: "Serializable Class Containing Sensitive Data",
125+
},
121126
"676": {
122127
ID: "676",
123128
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.",

‎issue/issue.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ var ruleToCWE = map[string]string{
6868
"G114": "676",
6969
"G115": "190",
7070
"G116": "838",
71+
"G117": "499",
7172
"G201": "89",
7273
"G202": "89",
7374
"G203": "79",

‎rules/rulelist.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList {
7777
{"G112", "Detect ReadHeaderTimeout not configured as a potential risk", NewSlowloris},
7878
{"G114", "Use of net/http serve function that has no support for setting timeouts", NewHTTPServeWithoutTimeouts},
7979
{"G116", "Detect Trojan Source attacks using bidirectional Unicode characters", NewTrojanSource},
80+
{"G117", "Potential exposure of secrets via JSON marshaling", NewSecretSerialization},
8081

8182
// injection
8283
{"G201", "SQL query construction using format string", NewSQLStrFormat},

‎rules/rules_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ var _ = Describe("gosec rules", func() {
111111
runner("G116", testutils.SampleCodeG116)
112112
})
113113

114+
It("should detect exported struct fields that may contain secrets and are JSON serializable", func() {
115+
runner("G117", testutils.SampleCodeG117)
116+
})
117+
114118
It("should detect sql injection via format strings", func() {
115119
runner("G201", testutils.SampleCodeG201)
116120
})

‎rules/secret_serialization.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package rules
2+
3+
import (
4+
"fmt"
5+
"go/ast"
6+
"reflect"
7+
"regexp"
8+
"strconv"
9+
"strings"
10+
11+
"github.com/securego/gosec/v2"
12+
"github.com/securego/gosec/v2/issue"
13+
)
14+
15+
type secretSerialization struct {
16+
issue.MetaData
17+
pattern *regexp.Regexp
18+
}
19+
20+
func (r *secretSerialization) ID() string {
21+
return r.MetaData.ID
22+
}
23+
24+
func (r *secretSerialization) Match(n ast.Node, ctx *gosec.Context) (*issue.Issue, error) {
25+
field, ok := n.(*ast.Field)
26+
if !ok || len(field.Names) == 0 {
27+
return nil, nil // skip embedded (anonymous) fields
28+
}
29+
30+
// Parse the JSON tag to determine behavior
31+
omitted := false
32+
jsonKey := ""
33+
34+
if field.Tag != nil {
35+
if tagVal, err := strconv.Unquote(field.Tag.Value); err == nil && tagVal != "" {
36+
st := reflect.StructTag(tagVal)
37+
if tag := st.Get("json"); tag != "" {
38+
if tag == "-" {
39+
omitted = true
40+
} else {
41+
// "name,omitempty" -> "name"
42+
// "-," -> "-" (A field literally named "-")
43+
parts := strings.SplitN(tag, ",", 2)
44+
jsonKey = parts[0]
45+
}
46+
}
47+
}
48+
}
49+
50+
if omitted {
51+
return nil, nil
52+
}
53+
54+
// Iterate over all names in this field definition
55+
// e.g., type T struct { Pass, Salt string }
56+
isSensitiveType := false
57+
switch t := field.Type.(type) {
58+
case *ast.Ident:
59+
if t.Name == "string" {
60+
isSensitiveType = true
61+
}
62+
case *ast.StarExpr:
63+
if ident, ok := t.X.(*ast.Ident); ok && ident.Name == "string" {
64+
isSensitiveType = true
65+
}
66+
case *ast.ArrayType:
67+
if star, ok := t.Elt.(*ast.StarExpr); ok {
68+
if ident, ok := star.X.(*ast.Ident); ok && ident.Name == "string" {
69+
isSensitiveType = true // []*string
70+
}
71+
} else if ident, ok := t.Elt.(*ast.Ident); ok {
72+
if ident.Name == "string" || ident.Name == "byte" {
73+
isSensitiveType = true // []string or []byte
74+
}
75+
}
76+
}
77+
78+
if !isSensitiveType {
79+
return nil, nil
80+
}
81+
82+
// Check each named exported field
83+
for _, nameIdent := range field.Names {
84+
fieldName := nameIdent.Name
85+
if fieldName == "_" || !ast.IsExported(fieldName) {
86+
continue
87+
}
88+
89+
effectiveKey := jsonKey
90+
if effectiveKey == "" {
91+
effectiveKey = fieldName
92+
}
93+
94+
if r.pattern.MatchString(fieldName) || r.pattern.MatchString(effectiveKey) {
95+
msg := fmt.Sprintf("Exported struct field %q (JSON key %q) matches secret pattern", fieldName, effectiveKey)
96+
return ctx.NewIssue(field, r.ID(), msg, r.Severity, r.Confidence), nil
97+
}
98+
}
99+
100+
return nil, nil
101+
}
102+
103+
func NewSecretSerialization(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
104+
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`
105+
106+
if val, ok := conf[id]; ok {
107+
if m, ok := val.(map[string]interface{}); ok {
108+
if p, ok := m["pattern"].(string); ok && p != "" {
109+
patternStr = p
110+
}
111+
}
112+
}
113+
114+
return &secretSerialization{
115+
pattern: regexp.MustCompile(patternStr),
116+
MetaData: issue.MetaData{
117+
ID: id,
118+
What: "Exported struct field appears to be a secret and is not ignored by JSON marshaling",
119+
Severity: issue.Medium,
120+
Confidence: issue.Medium,
121+
},
122+
}, []ast.Node{(*ast.Field)(nil)}
123+
}

‎testutils/g117_samples.go‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
// testutils/g117_samples.go
2+
package testutils
3+
4+
import "github.com/securego/gosec/v2"
5+
6+
var SampleCodeG117 = []CodeSample{
7+
// Positive: match on field name (default JSON key)
8+
{[]string{`
9+
package main
10+
11+
type Config struct {
12+
Password string
13+
}
14+
`}, 1, gosec.NewConfig()},
15+
{[]string{`
16+
package main
17+
18+
type Config struct {
19+
APIKey *string ` + "`json:\"api_key\"`" + `
20+
}
21+
`}, 1, gosec.NewConfig()},
22+
23+
{[]string{`
24+
package main
25+
26+
type Config struct {
27+
PrivateKey []byte ` + "`json:\"private_key\"`" + `
28+
}
29+
`}, 1, gosec.NewConfig()},
30+
31+
// Positive: match on field name (explicit non-sensitive JSON key)
32+
{[]string{`
33+
package main
34+
35+
type Config struct {
36+
Password string ` + "`json:\"text_field\"`" + `
37+
}
38+
`}, 1, gosec.NewConfig()},
39+
40+
// Positive: match on JSON key (non-sensitive field name)
41+
{[]string{`
42+
package main
43+
44+
type Config struct {
45+
SafeField string ` + "`json:\"api_key\"`" + `
46+
}
47+
`}, 1, gosec.NewConfig()},
48+
49+
// Positive: match on both
50+
{[]string{`
51+
package main
52+
53+
type Config struct {
54+
Token string ` + "`json:\"auth_token\"`" + `
55+
}
56+
`}, 1, gosec.NewConfig()},
57+
58+
// Positive: snake/hyphen variants in JSON key
59+
{[]string{`
60+
package main
61+
62+
type Config struct {
63+
Key string ` + "`json:\"access-key\"`" + `
64+
}
65+
`}, 1, gosec.NewConfig()},
66+
67+
// Positive: empty json tag part falls back to field name
68+
{[]string{`
69+
package main
70+
71+
type Config struct {
72+
Secret string ` + "`json:\",omitempty\"`" + `
73+
}
74+
`}, 1, gosec.NewConfig()},
75+
76+
// Positive: plural forms
77+
{[]string{`
78+
package main
79+
80+
type Config struct {
81+
ApiTokens []string
82+
}
83+
`}, 1, gosec.NewConfig()},
84+
85+
{[]string{`
86+
package main
87+
88+
type Config struct {
89+
RefreshTokens []string ` + "`json:\"refresh_tokens\"`" + `
90+
}
91+
`}, 1, gosec.NewConfig()},
92+
93+
{[]string{`
94+
package main
95+
96+
type Config struct {
97+
AccessTokens []*string
98+
}
99+
`}, 1, gosec.NewConfig()},
100+
101+
{[]string{`
102+
package main
103+
104+
type Config struct {
105+
CustomSecret string ` + "`json:\"my_custom_secret\"`" + `
106+
}
107+
`}, 1, func() gosec.Config {
108+
cfg := gosec.NewConfig()
109+
cfg.Set("G117", map[string]interface{}{
110+
"pattern": "(?i)custom[_-]?secret",
111+
})
112+
return cfg
113+
}()},
114+
115+
// Negative: json:"-" (omitted)
116+
{[]string{`
117+
package main
118+
119+
type Config struct {
120+
Password string ` + "`json:\"-\"`" + `
121+
}
122+
`}, 0, gosec.NewConfig()},
123+
124+
// Negative: both field name and JSON key non-sensitive
125+
{[]string{`
126+
package main
127+
128+
type Config struct {
129+
UserID string ` + "`json:\"user_id\"`" + `
130+
}
131+
`}, 0, gosec.NewConfig()},
132+
133+
// Negative: unexported field
134+
{[]string{`
135+
package main
136+
137+
type Config struct {
138+
password string
139+
}
140+
`}, 0, gosec.NewConfig()},
141+
142+
// Negative: non-sensitive type (int) even with "token"
143+
{[]string{`
144+
package main
145+
146+
type Config struct {
147+
MaxTokens int
148+
}
149+
`}, 0, gosec.NewConfig()},
150+
151+
// Negative: non-secret plural slice (common FP like redaction placeholders)
152+
{[]string{`
153+
package main
154+
155+
type Config struct {
156+
RedactionTokens []string ` + "`json:\"redactionTokens,omitempty\"`" + `
157+
}
158+
`}, 0, gosec.NewConfig()},
159+
160+
// Negative: grouped fields, only one sensitive (should still flag the sensitive one)
161+
// Note: we expect 1 issue (for the sensitive field)
162+
{[]string{`
163+
package main
164+
165+
type Config struct {
166+
Safe, Password string
167+
}
168+
`}, 1, gosec.NewConfig()},
169+
170+
// Suppression: trailing line comment
171+
{[]string{`
172+
package main
173+
174+
type Config struct {
175+
Password string // #nosec G117
176+
}
177+
`}, 0, gosec.NewConfig()},
178+
179+
// Suppression: line comment above field
180+
{[]string{`
181+
package main
182+
183+
type Config struct {
184+
// #nosec G117 -- false positive
185+
Password string
186+
}
187+
`}, 0, gosec.NewConfig()},
188+
189+
// Suppression: trailing with justification
190+
{[]string{`
191+
package main
192+
193+
type Config struct {
194+
APIKey string ` + "`json:\"api_key\"`" + ` // #nosec G117 -- public key
195+
}
196+
`}, 0, gosec.NewConfig()},
197+
}

0 commit comments

Comments
 (0)