mirror of
https://github.com/securego/gosec.git
synced 2026-06-20 00:15:59 +02:00
Add G710 rule for open redirect via taint analysis (#1654)
This commit is contained in:
@@ -101,6 +101,7 @@
|
|||||||
- G707 — SMTP command/header injection via taint analysis (**Taint**)
|
- G707 — SMTP command/header injection via taint analysis (**Taint**)
|
||||||
- G708 — Server-side template injection via `text/template` (**Taint**)
|
- G708 — Server-side template injection via `text/template` (**Taint**)
|
||||||
- G709 — Unsafe deserialization of untrusted data (**Taint**)
|
- G709 — Unsafe deserialization of untrusted data (**Taint**)
|
||||||
|
- G710 — Open redirect via taint analysis (**Taint**)
|
||||||
|
|
||||||
_Note: Implementation types used in this document:_
|
_Note: Implementation types used in this document:_
|
||||||
- **AST**: rule implemented in `rules/` and evaluated on AST patterns
|
- **AST**: rule implemented in `rules/` and evaluated on AST patterns
|
||||||
|
|||||||
@@ -134,5 +134,9 @@ var _ = Describe("gosec analyzers", func() {
|
|||||||
It("should detect unsafe deserialization via taint analysis", func() {
|
It("should detect unsafe deserialization via taint analysis", func() {
|
||||||
runner("G709", testutils.SampleCodeG709)
|
runner("G709", testutils.SampleCodeG709)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
It("should detect open redirect via taint analysis", func() {
|
||||||
|
runner("G710", testutils.SampleCodeG710)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -96,6 +96,13 @@ var (
|
|||||||
CWE: "CWE-502",
|
CWE: "CWE-502",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
OpenRedirectRule = taint.RuleInfo{
|
||||||
|
ID: "G710",
|
||||||
|
Description: "Open redirect: user-controlled URL flows into http.Redirect",
|
||||||
|
Severity: "MEDIUM",
|
||||||
|
CWE: "CWE-601",
|
||||||
|
}
|
||||||
|
|
||||||
FormParsingLimitRule = taint.RuleInfo{
|
FormParsingLimitRule = taint.RuleInfo{
|
||||||
ID: "G120",
|
ID: "G120",
|
||||||
Description: "Unbounded multipart form parsing can cause memory exhaustion",
|
Description: "Unbounded multipart form parsing can cause memory exhaustion",
|
||||||
@@ -162,6 +169,7 @@ var defaultAnalyzers = []AnalyzerDefinition{
|
|||||||
{"G707", "SMTP command/header injection via taint analysis", newSMTPInjectionAnalyzer},
|
{"G707", "SMTP command/header injection via taint analysis", newSMTPInjectionAnalyzer},
|
||||||
{"G708", "Server-side template injection via taint analysis", newSSTIAnalyzer},
|
{"G708", "Server-side template injection via taint analysis", newSSTIAnalyzer},
|
||||||
{"G709", "Unsafe deserialization of untrusted data via taint analysis", newUnsafeDeserializationAnalyzer},
|
{"G709", "Unsafe deserialization of untrusted data via taint analysis", newUnsafeDeserializationAnalyzer},
|
||||||
|
{"G710", "Open redirect via taint analysis", newOpenRedirectAnalyzer},
|
||||||
}
|
}
|
||||||
|
|
||||||
// Generate the list of analyzers to use
|
// Generate the list of analyzers to use
|
||||||
@@ -199,6 +207,7 @@ func DefaultTaintAnalyzers() []*analysis.Analyzer {
|
|||||||
sstiConfig := SSTI()
|
sstiConfig := SSTI()
|
||||||
deserConfig := UnsafeDeserialization()
|
deserConfig := UnsafeDeserialization()
|
||||||
formConfig := FormParsingLimits()
|
formConfig := FormParsingLimits()
|
||||||
|
openRedirectConfig := OpenRedirect()
|
||||||
|
|
||||||
return []*analysis.Analyzer{
|
return []*analysis.Analyzer{
|
||||||
taint.NewGosecAnalyzer(&SQLInjectionRule, &sqlConfig),
|
taint.NewGosecAnalyzer(&SQLInjectionRule, &sqlConfig),
|
||||||
@@ -211,5 +220,6 @@ func DefaultTaintAnalyzers() []*analysis.Analyzer {
|
|||||||
taint.NewGosecAnalyzer(&SSTIRule, &sstiConfig),
|
taint.NewGosecAnalyzer(&SSTIRule, &sstiConfig),
|
||||||
taint.NewGosecAnalyzer(&UnsafeDeserializationRule, &deserConfig),
|
taint.NewGosecAnalyzer(&UnsafeDeserializationRule, &deserConfig),
|
||||||
taint.NewGosecAnalyzer(&FormParsingLimitRule, &formConfig),
|
taint.NewGosecAnalyzer(&FormParsingLimitRule, &formConfig),
|
||||||
|
taint.NewGosecAnalyzer(&OpenRedirectRule, &openRedirectConfig),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -80,6 +80,12 @@ func TestTaintAnalyzerConstructors(t *testing.T) {
|
|||||||
id: "G709",
|
id: "G709",
|
||||||
description: "Unsafe deserialization of untrusted data via taint analysis",
|
description: "Unsafe deserialization of untrusted data via taint analysis",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "OpenRedirect",
|
||||||
|
constructor: newOpenRedirectAnalyzer,
|
||||||
|
id: "G710",
|
||||||
|
description: "Open redirect via taint analysis",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "FormParsingLimit",
|
name: "FormParsingLimit",
|
||||||
constructor: newFormParsingLimitAnalyzer,
|
constructor: newFormParsingLimitAnalyzer,
|
||||||
@@ -119,7 +125,7 @@ func TestTaintAnalyzerConstructors(t *testing.T) {
|
|||||||
|
|
||||||
// TestDefaultAnalyzersIncludeTaint tests that default analyzers include taint rules.
|
// TestDefaultAnalyzersIncludeTaint tests that default analyzers include taint rules.
|
||||||
func TestDefaultAnalyzersIncludeTaint(t *testing.T) {
|
func TestDefaultAnalyzersIncludeTaint(t *testing.T) {
|
||||||
expectedTaintIDs := []string{"G701", "G702", "G703", "G704", "G705", "G706", "G707", "G708", "G709"}
|
expectedTaintIDs := []string{"G701", "G702", "G703", "G704", "G705", "G706", "G707", "G708", "G709", "G710"}
|
||||||
|
|
||||||
found := make(map[string]bool)
|
found := make(map[string]bool)
|
||||||
for _, def := range defaultAnalyzers {
|
for _, def := range defaultAnalyzers {
|
||||||
@@ -137,7 +143,7 @@ func TestDefaultAnalyzersIncludeTaint(t *testing.T) {
|
|||||||
func TestGenerateIncludesTaintAnalyzers(t *testing.T) {
|
func TestGenerateIncludesTaintAnalyzers(t *testing.T) {
|
||||||
analyzerList := Generate(false)
|
analyzerList := Generate(false)
|
||||||
|
|
||||||
expectedTaintIDs := []string{"G701", "G702", "G703", "G704", "G705", "G706", "G707", "G708", "G709"}
|
expectedTaintIDs := []string{"G701", "G702", "G703", "G704", "G705", "G706", "G707", "G708", "G709", "G710"}
|
||||||
|
|
||||||
for _, id := range expectedTaintIDs {
|
for _, id := range expectedTaintIDs {
|
||||||
if _, ok := analyzerList.Analyzers[id]; !ok {
|
if _, ok := analyzerList.Analyzers[id]; !ok {
|
||||||
@@ -302,7 +308,7 @@ func TestAnalyzerList_AnalyzersInfo(t *testing.T) {
|
|||||||
func TestDefaultTaintAnalyzers(t *testing.T) {
|
func TestDefaultTaintAnalyzers(t *testing.T) {
|
||||||
analyzers := DefaultTaintAnalyzers()
|
analyzers := DefaultTaintAnalyzers()
|
||||||
|
|
||||||
expectedCount := 10 // SQL, Command, Path, SSRF, XSS, Log, SMTP, SSTI, Deserialization, FormParsing
|
expectedCount := 11 // SQL, Command, Path, SSRF, XSS, Log, SMTP, SSTI, Deserialization, FormParsing, OpenRedirect
|
||||||
if len(analyzers) != expectedCount {
|
if len(analyzers) != expectedCount {
|
||||||
t.Errorf("Expected %d taint analyzers, got %d", expectedCount, len(analyzers))
|
t.Errorf("Expected %d taint analyzers, got %d", expectedCount, len(analyzers))
|
||||||
}
|
}
|
||||||
@@ -317,6 +323,7 @@ func TestDefaultTaintAnalyzers(t *testing.T) {
|
|||||||
"G707": false,
|
"G707": false,
|
||||||
"G708": false,
|
"G708": false,
|
||||||
"G709": false,
|
"G709": false,
|
||||||
|
"G710": false,
|
||||||
"G120": false,
|
"G120": false,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
// (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 analyzers
|
||||||
|
|
||||||
|
import (
|
||||||
|
"golang.org/x/tools/go/analysis"
|
||||||
|
|
||||||
|
"github.com/securego/gosec/v2/taint"
|
||||||
|
)
|
||||||
|
|
||||||
|
// OpenRedirect returns a configuration for detecting open-redirect vulnerabilities
|
||||||
|
// where user-controlled data flows into the URL argument of net/http.Redirect.
|
||||||
|
// See CWE-601.
|
||||||
|
func OpenRedirect() taint.Config {
|
||||||
|
return taint.Config{
|
||||||
|
Sources: []taint.Source{
|
||||||
|
// Type sources: tainted when received as parameters from external callers.
|
||||||
|
// Any read from a *http.Request (FormValue, URL.Query().Get, Cookie, etc.)
|
||||||
|
// propagates taint through the existing receiver-based logic in isTainted.
|
||||||
|
{Package: "net/http", Name: "Request", Pointer: true},
|
||||||
|
{Package: "net/url", Name: "URL", Pointer: true},
|
||||||
|
{Package: "net/url", Name: "Values"},
|
||||||
|
},
|
||||||
|
Sinks: []taint.Sink{
|
||||||
|
// http.Redirect(w, r, url, code): only the URL string (arg index 2)
|
||||||
|
// is the redirect target. Skipping arg 1 (*http.Request) prevents the
|
||||||
|
// receiver itself from being treated as a tainted sink argument.
|
||||||
|
{Package: "net/http", Method: "Redirect", CheckArgs: []int{2}},
|
||||||
|
},
|
||||||
|
Sanitizers: []taint.Sanitizer{
|
||||||
|
// url.PathEscape / QueryEscape neutralize untrusted path or query
|
||||||
|
// fragments embedded into a hard-coded base URL.
|
||||||
|
{Package: "net/url", Method: "PathEscape"},
|
||||||
|
{Package: "net/url", Method: "QueryEscape"},
|
||||||
|
|
||||||
|
// Numeric conversions cannot produce a URL host or scheme.
|
||||||
|
{Package: "strconv", Method: "Atoi"},
|
||||||
|
{Package: "strconv", Method: "Itoa"},
|
||||||
|
{Package: "strconv", Method: "ParseInt"},
|
||||||
|
{Package: "strconv", Method: "ParseUint"},
|
||||||
|
{Package: "strconv", Method: "FormatInt"},
|
||||||
|
{Package: "strconv", Method: "FormatUint"},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// newOpenRedirectAnalyzer creates an analyzer for detecting open-redirect
|
||||||
|
// vulnerabilities via taint analysis (G710).
|
||||||
|
func newOpenRedirectAnalyzer(id string, description string) *analysis.Analyzer {
|
||||||
|
config := OpenRedirect()
|
||||||
|
rule := OpenRedirectRule
|
||||||
|
rule.ID = id
|
||||||
|
rule.Description = description
|
||||||
|
return taint.NewGosecAnalyzer(&rule, &config)
|
||||||
|
}
|
||||||
@@ -112,6 +112,7 @@ var ruleToCWE = map[string]string{
|
|||||||
"G704": "918",
|
"G704": "918",
|
||||||
"G705": "79",
|
"G705": "79",
|
||||||
"G706": "117",
|
"G706": "117",
|
||||||
|
"G710": "601",
|
||||||
}
|
}
|
||||||
|
|
||||||
// Issue is returned by a gosec rule if it discovers an issue with the scanned code.
|
// Issue is returned by a gosec rule if it discovers an issue with the scanned code.
|
||||||
|
|||||||
@@ -0,0 +1,68 @@
|
|||||||
|
package testutils
|
||||||
|
|
||||||
|
import "github.com/securego/gosec/v2"
|
||||||
|
|
||||||
|
// SampleCodeG710 - Open redirect via taint analysis
|
||||||
|
var SampleCodeG710 = []CodeSample{
|
||||||
|
// Positive: query parameter flows directly into http.Redirect URL.
|
||||||
|
{[]string{`
|
||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
)
|
||||||
|
|
||||||
|
func handler(w http.ResponseWriter, r *http.Request) {
|
||||||
|
target := r.URL.Query().Get("next")
|
||||||
|
http.Redirect(w, r, target, http.StatusFound)
|
||||||
|
}
|
||||||
|
`}, 1, gosec.NewConfig()},
|
||||||
|
|
||||||
|
// Positive: form value concatenated into a redirect target.
|
||||||
|
{[]string{`
|
||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
)
|
||||||
|
|
||||||
|
func handler(w http.ResponseWriter, r *http.Request) {
|
||||||
|
dest := r.FormValue("redirect")
|
||||||
|
http.Redirect(w, r, "/proxy?to="+dest, http.StatusSeeOther)
|
||||||
|
}
|
||||||
|
`}, 1, gosec.NewConfig()},
|
||||||
|
|
||||||
|
// Negative: redirect to a constant URL — never tainted.
|
||||||
|
{[]string{`
|
||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
)
|
||||||
|
|
||||||
|
func handler(w http.ResponseWriter, r *http.Request) {
|
||||||
|
http.Redirect(w, r, "/dashboard", http.StatusFound)
|
||||||
|
}
|
||||||
|
`}, 0, gosec.NewConfig()},
|
||||||
|
|
||||||
|
// Negative: redirect target derived from numeric conversion of user input
|
||||||
|
// (strconv.Atoi sanitizer strips any redirect payload from the string).
|
||||||
|
{[]string{`
|
||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"strconv"
|
||||||
|
)
|
||||||
|
|
||||||
|
func handler(w http.ResponseWriter, r *http.Request) {
|
||||||
|
id, err := strconv.Atoi(r.URL.Query().Get("id"))
|
||||||
|
if err != nil {
|
||||||
|
http.Error(w, "bad id", http.StatusBadRequest)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
http.Redirect(w, r, fmt.Sprintf("/users/%d", id), http.StatusFound)
|
||||||
|
}
|
||||||
|
`}, 0, gosec.NewConfig()},
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user