From 4ead098510926e1015958a36dc966bfcb7f6ee11 Mon Sep 17 00:00:00 2001 From: Ravi Sastry Kadali Date: Sun, 26 Apr 2026 00:38:45 -0700 Subject: [PATCH] Add G710 rule for open redirect via taint analysis (#1654) --- RULES.md | 1 + analyzers/analyzers_test.go | 4 ++ analyzers/analyzerslist.go | 10 +++++ analyzers/analyzerslist_test.go | 13 +++++-- analyzers/openredirect.go | 67 ++++++++++++++++++++++++++++++++ issue/issue.go | 1 + testutils/g710_samples.go | 68 +++++++++++++++++++++++++++++++++ 7 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 analyzers/openredirect.go create mode 100644 testutils/g710_samples.go diff --git a/RULES.md b/RULES.md index 3afa751..badc3b1 100644 --- a/RULES.md +++ b/RULES.md @@ -101,6 +101,7 @@ - G707 — SMTP command/header injection via taint analysis (**Taint**) - G708 — Server-side template injection via `text/template` (**Taint**) - G709 — Unsafe deserialization of untrusted data (**Taint**) +- G710 — Open redirect via taint analysis (**Taint**) _Note: Implementation types used in this document:_ - **AST**: rule implemented in `rules/` and evaluated on AST patterns diff --git a/analyzers/analyzers_test.go b/analyzers/analyzers_test.go index 42ef4ad..461c0b0 100644 --- a/analyzers/analyzers_test.go +++ b/analyzers/analyzers_test.go @@ -134,5 +134,9 @@ var _ = Describe("gosec analyzers", func() { It("should detect unsafe deserialization via taint analysis", func() { runner("G709", testutils.SampleCodeG709) }) + + It("should detect open redirect via taint analysis", func() { + runner("G710", testutils.SampleCodeG710) + }) }) }) diff --git a/analyzers/analyzerslist.go b/analyzers/analyzerslist.go index cedc4cf..0a541e9 100644 --- a/analyzers/analyzerslist.go +++ b/analyzers/analyzerslist.go @@ -96,6 +96,13 @@ var ( 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{ ID: "G120", 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}, {"G708", "Server-side template injection via taint analysis", newSSTIAnalyzer}, {"G709", "Unsafe deserialization of untrusted data via taint analysis", newUnsafeDeserializationAnalyzer}, + {"G710", "Open redirect via taint analysis", newOpenRedirectAnalyzer}, } // Generate the list of analyzers to use @@ -199,6 +207,7 @@ func DefaultTaintAnalyzers() []*analysis.Analyzer { sstiConfig := SSTI() deserConfig := UnsafeDeserialization() formConfig := FormParsingLimits() + openRedirectConfig := OpenRedirect() return []*analysis.Analyzer{ taint.NewGosecAnalyzer(&SQLInjectionRule, &sqlConfig), @@ -211,5 +220,6 @@ func DefaultTaintAnalyzers() []*analysis.Analyzer { taint.NewGosecAnalyzer(&SSTIRule, &sstiConfig), taint.NewGosecAnalyzer(&UnsafeDeserializationRule, &deserConfig), taint.NewGosecAnalyzer(&FormParsingLimitRule, &formConfig), + taint.NewGosecAnalyzer(&OpenRedirectRule, &openRedirectConfig), } } diff --git a/analyzers/analyzerslist_test.go b/analyzers/analyzerslist_test.go index 16d3131..d78bc60 100644 --- a/analyzers/analyzerslist_test.go +++ b/analyzers/analyzerslist_test.go @@ -80,6 +80,12 @@ func TestTaintAnalyzerConstructors(t *testing.T) { id: "G709", description: "Unsafe deserialization of untrusted data via taint analysis", }, + { + name: "OpenRedirect", + constructor: newOpenRedirectAnalyzer, + id: "G710", + description: "Open redirect via taint analysis", + }, { name: "FormParsingLimit", constructor: newFormParsingLimitAnalyzer, @@ -119,7 +125,7 @@ func TestTaintAnalyzerConstructors(t *testing.T) { // TestDefaultAnalyzersIncludeTaint tests that default analyzers include taint rules. 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) for _, def := range defaultAnalyzers { @@ -137,7 +143,7 @@ func TestDefaultAnalyzersIncludeTaint(t *testing.T) { func TestGenerateIncludesTaintAnalyzers(t *testing.T) { 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 { if _, ok := analyzerList.Analyzers[id]; !ok { @@ -302,7 +308,7 @@ func TestAnalyzerList_AnalyzersInfo(t *testing.T) { func TestDefaultTaintAnalyzers(t *testing.T) { 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 { t.Errorf("Expected %d taint analyzers, got %d", expectedCount, len(analyzers)) } @@ -317,6 +323,7 @@ func TestDefaultTaintAnalyzers(t *testing.T) { "G707": false, "G708": false, "G709": false, + "G710": false, "G120": false, } diff --git a/analyzers/openredirect.go b/analyzers/openredirect.go new file mode 100644 index 0000000..bcdddc9 --- /dev/null +++ b/analyzers/openredirect.go @@ -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) +} diff --git a/issue/issue.go b/issue/issue.go index 216e119..837bbfa 100644 --- a/issue/issue.go +++ b/issue/issue.go @@ -112,6 +112,7 @@ var ruleToCWE = map[string]string{ "G704": "918", "G705": "79", "G706": "117", + "G710": "601", } // Issue is returned by a gosec rule if it discovers an issue with the scanned code. diff --git a/testutils/g710_samples.go b/testutils/g710_samples.go new file mode 100644 index 0000000..a619fd4 --- /dev/null +++ b/testutils/g710_samples.go @@ -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()}, +}