Skip to content

Commit 36ba72b

Browse files
ccojocarccojocar
authored
Add G121 analyzer for unsafe CORS bypass patterns in CrossOriginProtection (#1521)
* Add G121 analyzer for unsafe CORS bypass patterns in CrossOriginProtection Adds new SSA-only analyzer G121 to detect unsafe usage of AddInsecureBypassPattern in net/http.CrossOriginProtection. Flags: overbroad static bypass patterns (for example /, /*, empty/wildcard-like values), request-derived dynamic bypass patterns. Wires G121 into default analyzer registration, analyzer test suite, CWE mapping (CWE-346), README rule list, and dedicated sample fixtures. Includes a narrow lint suppression for a G101 false positive in analyzer message constants. Validation: analyzer tests pass and golangci-lint reports 0 issues. Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch> * Ignore false positive warnings Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch> --------- Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
1 parent 238f982 commit 36ba72b

File tree

6 files changed

+282
-0
lines changed

6 files changed

+282
-0
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ directory you can supply `./...` as the input argument.
199199
- G118: Context propagation failure leading to goroutine/resource leaks
200200
- G119: Unsafe redirect policy may propagate sensitive headers
201201
- G120: Unbounded form parsing in HTTP handlers can cause memory exhaustion
202+
- G121: Unsafe CrossOriginProtection bypass patterns
202203
- G201: SQL query construction using format string
203204
- G202: SQL query construction using string concatenation
204205
- G203: Use of unescaped data in HTML templates

‎analyzers/analyzers_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ var _ = Describe("gosec analyzers", func() {
7171
runner("G120", testutils.SampleCodeG120)
7272
})
7373

74+
It("should detect unsafe CORS bypass patterns", func() {
75+
runner("G121", testutils.SampleCodeG121)
76+
})
77+
7478
It("should detect hardcoded nonce/IV", func() {
7579
runner("G407", testutils.SampleCodeG407)
7680
})

‎analyzers/analyzerslist.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ var defaultAnalyzers = []AnalyzerDefinition{
118118
{"G118", "Context propagation failure leading to goroutine/resource leaks", newContextPropagationAnalyzer},
119119
{"G119", "Unsafe redirect policy may propagate sensitive headers", newRedirectHeaderPropagationAnalyzer},
120120
{"G120", "Unbounded form parsing in HTTP handlers can cause memory exhaustion", newFormParsingLimitAnalyzer},
121+
{"G121", "Unsafe CrossOriginProtection bypass patterns", newCORSBypassPatternAnalyzer},
121122
{"G602", "Possible slice bounds out of range", newSliceBoundsAnalyzer},
122123
{"G407", "Use of hardcoded IV/nonce for encryption", newHardCodedNonce},
123124
{"G408", "Stateful misuse of ssh.PublicKeyCallback leading to auth bypass", newSSHCallbackAnalyzer},

‎analyzers/cors_bypass_pattern.go‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
// (c) Copyright gosec's authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package analyzers
16+
17+
import (
18+
"go/token"
19+
"go/types"
20+
"strings"
21+
22+
"golang.org/x/tools/go/analysis"
23+
"golang.org/x/tools/go/analysis/passes/buildssa"
24+
"golang.org/x/tools/go/ssa"
25+
26+
"github.com/securego/gosec/v2/internal/ssautil"
27+
"github.com/securego/gosec/v2/issue"
28+
)
29+
30+
const (
31+
msgOverbroadBypassPattern = "Overbroad AddInsecureBypassPattern disables cross-origin protections for too many paths" // #nosec G101 -- Message string includes API name, not credentials.
32+
msgRequestBypassPattern = "AddInsecureBypassPattern argument derived from request data can allow bypass of cross-origin protections" // #nosec G101 -- Message string includes API name, not credentials.
33+
)
34+
35+
func newCORSBypassPatternAnalyzer(id string, description string) *analysis.Analyzer {
36+
return &analysis.Analyzer{
37+
Name: id,
38+
Doc: description,
39+
Run: runCORSBypassPatternAnalysis,
40+
Requires: []*analysis.Analyzer{buildssa.Analyzer},
41+
}
42+
}
43+
44+
func runCORSBypassPatternAnalysis(pass *analysis.Pass) (any, error) {
45+
ssaResult, err := ssautil.GetSSAResult(pass)
46+
if err != nil {
47+
return nil, err
48+
}
49+
50+
issuesByPos := make(map[token.Pos]*issue.Issue)
51+
52+
for _, fn := range collectAnalyzerFunctions(ssaResult.SSA.SrcFuncs) {
53+
requestParam := findHTTPRequestParam(fn)
54+
55+
for _, block := range fn.Blocks {
56+
for _, instr := range block.Instrs {
57+
callInstr, ok := instr.(ssa.CallInstruction)
58+
if !ok {
59+
continue
60+
}
61+
62+
common := callInstr.Common()
63+
if common == nil {
64+
continue
65+
}
66+
67+
if !isAddInsecureBypassPatternCall(common) {
68+
continue
69+
}
70+
71+
if len(common.Args) < 2 {
72+
continue
73+
}
74+
75+
patternArg := common.Args[1]
76+
if pattern, ok := extractStringValue(patternArg, 0); ok {
77+
if isOverbroadBypassPattern(pattern) {
78+
addG121Issue(issuesByPos, pass, instr.Pos(), msgOverbroadBypassPattern, issue.High, issue.High)
79+
}
80+
continue
81+
}
82+
83+
if requestParam != nil && valueDependsOn(patternArg, requestParam, 0) {
84+
addG121Issue(issuesByPos, pass, instr.Pos(), msgRequestBypassPattern, issue.High, issue.Medium)
85+
}
86+
}
87+
}
88+
}
89+
90+
if len(issuesByPos) == 0 {
91+
return nil, nil
92+
}
93+
94+
issues := make([]*issue.Issue, 0, len(issuesByPos))
95+
for _, i := range issuesByPos {
96+
issues = append(issues, i)
97+
}
98+
99+
return issues, nil
100+
}
101+
102+
func addG121Issue(issues map[token.Pos]*issue.Issue, pass *analysis.Pass, pos token.Pos, what string, severity issue.Score, confidence issue.Score) {
103+
if pos == token.NoPos {
104+
return
105+
}
106+
if _, exists := issues[pos]; exists {
107+
return
108+
}
109+
issues[pos] = newIssue(pass.Analyzer.Name, what, pass.Fset, pos, severity, confidence)
110+
}
111+
112+
func findHTTPRequestParam(fn *ssa.Function) *ssa.Parameter {
113+
if fn == nil {
114+
return nil
115+
}
116+
for _, param := range fn.Params {
117+
if param == nil {
118+
continue
119+
}
120+
if isHTTPRequestPointerType(param.Type()) {
121+
return param
122+
}
123+
}
124+
return nil
125+
}
126+
127+
func isAddInsecureBypassPatternCall(call *ssa.CallCommon) bool {
128+
callee := call.StaticCallee()
129+
if callee == nil || callee.Name() != "AddInsecureBypassPattern" {
130+
return false
131+
}
132+
133+
sig := callee.Signature
134+
if sig == nil || sig.Recv() == nil {
135+
return false
136+
}
137+
138+
return isCrossOriginProtectionType(sig.Recv().Type())
139+
}
140+
141+
func isCrossOriginProtectionType(t types.Type) bool {
142+
if ptr, ok := t.(*types.Pointer); ok {
143+
t = ptr.Elem()
144+
}
145+
146+
named, ok := t.(*types.Named)
147+
if !ok {
148+
return false
149+
}
150+
obj := named.Obj()
151+
if obj == nil || obj.Name() != "CrossOriginProtection" {
152+
return false
153+
}
154+
pkg := obj.Pkg()
155+
return pkg != nil && pkg.Path() == "net/http"
156+
}
157+
158+
func extractStringValue(v ssa.Value, depth int) (string, bool) {
159+
if v == nil || depth > MaxDepth {
160+
return "", false
161+
}
162+
163+
if value := extractStringConst(v); value != "" {
164+
return value, true
165+
}
166+
167+
switch x := v.(type) {
168+
case *ssa.ChangeType:
169+
return extractStringValue(x.X, depth+1)
170+
case *ssa.MakeInterface:
171+
return extractStringValue(x.X, depth+1)
172+
case *ssa.TypeAssert:
173+
return extractStringValue(x.X, depth+1)
174+
case *ssa.Phi:
175+
if len(x.Edges) == 0 {
176+
return "", false
177+
}
178+
var candidate string
179+
for _, edge := range x.Edges {
180+
val, ok := extractStringValue(edge, depth+1)
181+
if !ok {
182+
return "", false
183+
}
184+
if candidate == "" {
185+
candidate = val
186+
continue
187+
}
188+
if candidate != val {
189+
return "", false
190+
}
191+
}
192+
return candidate, true
193+
}
194+
195+
return "", false
196+
}
197+
198+
func isOverbroadBypassPattern(pattern string) bool {
199+
normalized := strings.TrimSpace(pattern)
200+
switch normalized {
201+
case "", "/", "*", "/*", "/**", ".*", "/.*":
202+
return true
203+
default:
204+
return false
205+
}
206+
}

‎issue/issue.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ var ruleToCWE = map[string]string{
7272
"G118": "400",
7373
"G119": "200",
7474
"G120": "400",
75+
"G121": "346",
7576
"G201": "89",
7677
"G202": "89",
7778
"G203": "79",

‎testutils/g121_samples.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package testutils
2+
3+
import "github.com/securego/gosec/v2"
4+
5+
// SampleCodeG121 - Unsafe CORS bypass patterns via CrossOriginProtection
6+
var SampleCodeG121 = []CodeSample{
7+
// Vulnerable: overbroad root bypass
8+
{[]string{`
9+
package main
10+
11+
import "net/http"
12+
13+
func setup() {
14+
var cop http.CrossOriginProtection
15+
cop.AddInsecureBypassPattern("/")
16+
}
17+
`}, 1, gosec.NewConfig()},
18+
19+
// Vulnerable: overbroad wildcard bypass
20+
{[]string{`
21+
package main
22+
23+
import "net/http"
24+
25+
func setup() {
26+
var cop http.CrossOriginProtection
27+
cop.AddInsecureBypassPattern("/*")
28+
}
29+
`}, 1, gosec.NewConfig()},
30+
31+
// Vulnerable: user-controlled bypass pattern from request data
32+
{[]string{`
33+
package main
34+
35+
import "net/http"
36+
37+
func handler(w http.ResponseWriter, r *http.Request) {
38+
_ = w
39+
var cop http.CrossOriginProtection
40+
pattern := r.URL.Query().Get("bypass")
41+
cop.AddInsecureBypassPattern(pattern)
42+
}
43+
`}, 1, gosec.NewConfig()},
44+
45+
// Safe: narrow static bypass
46+
{[]string{`
47+
package main
48+
49+
import "net/http"
50+
51+
func setup() {
52+
var cop http.CrossOriginProtection
53+
cop.AddInsecureBypassPattern("/healthz")
54+
}
55+
`}, 0, gosec.NewConfig()},
56+
57+
// Safe: multiple narrow static bypasses
58+
{[]string{`
59+
package main
60+
61+
import "net/http"
62+
63+
func setup() {
64+
var cop http.CrossOriginProtection
65+
cop.AddInsecureBypassPattern("/status")
66+
cop.AddInsecureBypassPattern("/metrics")
67+
}
68+
`}, 0, gosec.NewConfig()},
69+
}

0 commit comments

Comments
 (0)