sec8: validate zeroconf port to break CodeQL taint chain (alerts 134/135/136)

Add validateZcPort alongside validateZcHost: the strconv.Atoi→Itoa
round-trip produces a sanitised integer string that CodeQL no longer
considers tainted, closing the remaining go/request-forgery findings
at zeroconf.go:263, :336, :413.

Also rejects clearly invalid inputs (non-numeric, out-of-range) that
would previously have produced a silently broken URL.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-25 21:17:57 +02:00
co-authored by Claude Sonnet 4.6
parent 6f488c2016
commit bf3466d5d9
2 changed files with 93 additions and 11 deletions
+40 -6
View File
@@ -20,6 +20,7 @@ import (
"net"
"net/http"
"net/url"
"strconv"
"strings"
"time"
)
@@ -212,17 +213,41 @@ func validateZcHost(host string) (net.IP, error) {
return ip, nil
}
// validateZcPort checks that port is a decimal integer in [1, 65535] and
// returns the canonical decimal string produced by strconv.Itoa. The
// round-trip through an integer breaks static-analysis taint chains while
// also rejecting strings like "0", "-1", or "foo" that would silently
// produce a broken URL.
// An empty port string is allowed and returned unchanged — callers that omit
// the port rely on the scheme default.
func validateZcPort(port string) (string, error) {
if port == "" {
return "", nil
}
p, err := strconv.Atoi(port)
if err != nil || p < 1 || p > 65535 {
return "", fmt.Errorf("zeroconf port %q must be a decimal integer in [1, 65535]", port)
}
return strconv.Itoa(p), nil
}
// buildZcBase constructs the ZeroConf base URL from a validated IP and port.
// The path is always the literal "/zc" — no user-supplied path component ever
// flows here, which is what satisfies CodeQL's go/request-forgery model.
// port may be empty, in which case the scheme default applies.
func buildZcBase(ip net.IP, port string) *url.URL {
func buildZcBase(ip net.IP, port string) (*url.URL, error) {
var host string
switch {
case port != "":
safePort, err := validateZcPort(port)
if err != nil {
return nil, err
}
// net.JoinHostPort brackets IPv6 addresses automatically.
host = net.JoinHostPort(ip.String(), port)
host = net.JoinHostPort(ip.String(), safePort)
case ip.To4() == nil:
// IPv6 address without a port must be bracketed in a URL host field.
host = "[" + ip.String() + "]"
@@ -234,7 +259,7 @@ func buildZcBase(ip net.IP, port string) *url.URL {
Scheme: "http",
Host: host,
Path: "/zc",
}
}, nil
}
// withAction returns the validated base URL with ?action=<action> appended.
@@ -256,7 +281,10 @@ func GetInfo(host, port string) ([]byte, error) {
return nil, fmt.Errorf("getInfo: %w", err)
}
base := buildZcBase(ip, port)
base, err := buildZcBase(ip, port)
if err != nil {
return nil, fmt.Errorf("getInfo: %w", err)
}
client := &http.Client{Timeout: 10 * time.Second}
@@ -303,7 +331,10 @@ func PushCredentials(host, port, username, accessToken string) error {
return fmt.Errorf("pushCredentials: %w", err)
}
base := buildZcBase(ip, port)
base, err := buildZcBase(ip, port)
if err != nil {
return fmt.Errorf("pushCredentials: %w", err)
}
speakerPublicKey, err := GetInfo(host, port)
if err != nil {
@@ -400,7 +431,10 @@ func pushSimplifiedToken(host, port, username, accessToken string) error {
return fmt.Errorf("pushSimplifiedToken: %w", err)
}
base := buildZcBase(ip, port)
base, err := buildZcBase(ip, port)
if err != nil {
return fmt.Errorf("pushSimplifiedToken: %w", err)
}
data := url.Values{}
data.Set("userName", username)
+53 -5
View File
@@ -365,17 +365,56 @@ func TestValidateZcHost(t *testing.T) {
}
}
func TestValidateZcPort(t *testing.T) {
cases := []struct {
input string
wantOut string
wantErr bool
}{
{"8200", "8200", false},
{"1", "1", false},
{"65535", "65535", false},
{"", "", false}, // empty → scheme default, not an error
{"0", "", true}, // below range
{"-1", "", true}, // negative
{"65536", "", true}, // above range
{"foo", "", true}, // non-numeric
{"8200x", "", true}, // trailing garbage
}
for _, tc := range cases {
t.Run(tc.input, func(t *testing.T) {
got, err := validateZcPort(tc.input)
if tc.wantErr {
if err == nil {
t.Errorf("validateZcPort(%q) succeeded with %q, want error", tc.input, got)
}
return
}
if err != nil {
t.Fatalf("validateZcPort(%q) returned error %v, want success", tc.input, err)
}
if got != tc.wantOut {
t.Errorf("validateZcPort(%q) = %q, want %q", tc.input, got, tc.wantOut)
}
})
}
}
func TestBuildZcBase(t *testing.T) {
cases := []struct {
name string
host string
port string
wantURL string
wantErr bool
}{
{"with port", "127.0.0.1", "8200", "http://127.0.0.1:8200/zc"},
{"without port", "192.168.1.1", "", "http://192.168.1.1/zc"},
{"ipv6 with port", "::1", "8200", "http://[::1]:8200/zc"},
{"ipv6 without port", "::1", "", "http://[::1]/zc"},
{"with port", "127.0.0.1", "8200", "http://127.0.0.1:8200/zc", false},
{"without port", "192.168.1.1", "", "http://192.168.1.1/zc", false},
{"ipv6 with port", "::1", "8200", "http://[::1]:8200/zc", false},
{"ipv6 without port", "::1", "", "http://[::1]/zc", false},
{"invalid port", "127.0.0.1", "foo", "", true},
{"port out of range", "127.0.0.1", "99999", "", true},
}
for _, tc := range cases {
@@ -384,7 +423,16 @@ func TestBuildZcBase(t *testing.T) {
if ip == nil {
t.Fatalf("test setup: net.ParseIP(%q) returned nil", tc.host)
}
got := buildZcBase(ip, tc.port)
got, err := buildZcBase(ip, tc.port)
if tc.wantErr {
if err == nil {
t.Errorf("buildZcBase(%q, %q) succeeded, want error", tc.host, tc.port)
}
return
}
if err != nil {
t.Fatalf("buildZcBase(%q, %q) returned error %v, want success", tc.host, tc.port, err)
}
if got.String() != tc.wantURL {
t.Errorf("buildZcBase(%q, %q) = %q, want %q", tc.host, tc.port, got.String(), tc.wantURL)
}