Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions taint/taint.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,23 +858,47 @@ func (a *Analyzer) isParameterTainted(param *ssa.Parameter, fn *ssa.Function, vi
}
}

// Check if parameter type is a source type.
// This is the ONLY place where type-based source matching should trigger
// automatic taint — because parameters represent data flowing IN from
// external callers we don't control.
if a.isSourceType(param.Type()) {
if paramIdx >= 0 && a.paramTaintCache != nil {
a.paramTaintCache[paramKey{fn: fn, paramIdx: paramIdx}] = true
}
return true
}

// Use call graph to find callers and check their arguments
if a.callGraph == nil {
// No call graph: fall back to type-based auto-taint for source-typed params
// (conservative — may produce false positives, but we have no callee info).
if a.isSourceType(param.Type()) {
if paramIdx >= 0 && a.paramTaintCache != nil {
a.paramTaintCache[paramKey{fn: fn, paramIdx: paramIdx}] = true
}
return true
}
return false
}

node := a.callGraph.Nodes[fn]

// Check if parameter type is a configured source type (e.g., *http.Request).
//
// Rationale: a parameter of type *http.Request is tainted only when it
// actually originates from an untrusted HTTP request. In practice that
// means the function is an entry point whose caller lives outside the
// package (e.g., an HTTP handler registered with net/http). When the
// function DOES have known callers inside the package we verify taint
// through those callers instead. This eliminates false positives for
// wrapper types like:
//
// func (c *NamedClient) Do(req *http.Request) (*http.Response, error) {
// return c.HTTPClient.Do(req) // was wrongly flagged
// }
//
// called only with requests whose URLs are compile-time constants.
if a.isSourceType(param.Type()) {
isEntryPoint := (node == nil || len(node.In) == 0)
if isEntryPoint {
if paramIdx >= 0 && a.paramTaintCache != nil {
a.paramTaintCache[paramKey{fn: fn, paramIdx: paramIdx}] = true
}
return true
}
// Has known callers — fall through to verify taint via those callers.
}

if node == nil {
return false
}
Expand Down
75 changes: 75 additions & 0 deletions testutils/g704_samples.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,80 @@ func handler(r *http.Request) {
target := r.URL.Query().Get("url")
http.Get(target) //nolint:errcheck
}
`}, 1, gosec.NewConfig()},
// Issue #1629: NamedClient wrapper delegates to http.Client.Do.
// Request built with constant URL — must NOT trigger G704.
{[]string{`
package main

import (
"context"
"fmt"
"net/http"
)

type HTTPDoer interface {
Do(req *http.Request) (*http.Response, error)
}

type NamedClient struct {
HTTPClient *http.Client
}

func (c *NamedClient) Do(req *http.Request) (*http.Response, error) {
req.Header.Set("User-Agent", "test-agent")
return c.HTTPClient.Do(req)
}

func doImport(httpDoer HTTPDoer) error {
ctx := context.Background()
req, err := http.NewRequestWithContext(ctx, http.MethodPost, "/import", http.NoBody)
if err != nil {
return fmt.Errorf("creating import POST: %w", err)
}
resp, err := httpDoer.Do(req)
if err != nil {
return fmt.Errorf("performing import POST: %w", err)
}
defer resp.Body.Close()
return nil
}
`}, 0, gosec.NewConfig()},
// Issue #1629 counterpart: URL from os.Getenv through wrapper MUST still fire.
{[]string{`
package main

import (
"context"
"net/http"
"os"
)

type HTTPDoer interface {
Do(req *http.Request) (*http.Response, error)
}

type NamedClient struct {
HTTPClient *http.Client
}

func (c *NamedClient) Do(req *http.Request) (*http.Response, error) {
return c.HTTPClient.Do(req)
}

func doImport(httpDoer HTTPDoer) error {
target := os.Getenv("IMPORT_URL")
ctx := context.Background()
req, err := http.NewRequestWithContext(ctx, http.MethodPost, target, http.NoBody)
if err != nil {
return err
}
resp, err := httpDoer.Do(req)
if err != nil {
return err
}
defer resp.Body.Close()
return nil
}
`}, 1, gosec.NewConfig()},
}
Loading