From 82eec17dfe64b42344d4fc95377f599e7f9a26d8 Mon Sep 17 00:00:00 2001 From: Snider Date: Thu, 16 Apr 2026 00:04:51 +0100 Subject: [PATCH] fix(cdp): harden debug transport and navigation Restrict DevTools access to loopback hosts, bound debug and WebSocket reads, validate navigation targets, and sanitise formatted console output. Co-Authored-By: Virgil --- actions.go | 8 +--- cdp.go | 65 +++++++++++++++++++++++++++++--- console.go | 28 +++++++++++++- webview.go | 12 +++++- webview_test.go | 98 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 196 insertions(+), 15 deletions(-) diff --git a/actions.go b/actions.go index 7165b54..28f4e26 100644 --- a/actions.go +++ b/actions.go @@ -42,13 +42,7 @@ type NavigateAction struct { // Execute performs the navigate action. func (a NavigateAction) Execute(ctx context.Context, wv *Webview) error { - _, err := wv.client.Call(ctx, "Page.navigate", map[string]any{ - "url": a.URL, - }) - if err != nil { - return coreerr.E("NavigateAction.Execute", "failed to navigate", err) - } - return wv.waitForLoad(ctx) + return wv.navigate(ctx, a.URL, "NavigateAction.Execute") } // WaitAction represents a wait action. diff --git a/cdp.go b/cdp.go index 8d9b825..e985e12 100644 --- a/cdp.go +++ b/cdp.go @@ -3,6 +3,7 @@ package webview import ( "context" + "io" "iter" "net" "net/http" @@ -20,6 +21,8 @@ import ( ) const debugEndpointTimeout = 10 * time.Second +const maxDebugResponseBytes = 1 << 20 +const maxCDPMessageBytes = 16 << 20 var ( defaultDebugHTTPClient = &http.Client{ @@ -141,6 +144,7 @@ func NewCDPClient(debugURL string) (*CDPClient, error) { if err != nil { return nil, coreerr.E("CDPClient.New", "failed to connect to WebSocket", err) } + conn.SetReadLimit(maxCDPMessageBytes) return newCDPClient(debugHTTPURL, wsURL, conn), nil } @@ -401,6 +405,7 @@ func GetVersion(debugURL string) (map[string]string, error) { func newCDPClient(debugHTTPURL *url.URL, wsURL string, conn *websocket.Conn) *CDPClient { ctx, cancel := context.WithCancel(context.Background()) baseCopy := *debugHTTPURL + conn.SetReadLimit(maxCDPMessageBytes) client := &CDPClient{ conn: conn, @@ -442,9 +447,24 @@ func parseDebugURL(raw string) (*url.URL, error) { if debugURL.Path != "/" { return nil, coreerr.E("CDPClient.parseDebugURL", "debug URL must point at the DevTools root", nil) } + if !isLoopbackHost(debugURL.Hostname()) { + return nil, coreerr.E("CDPClient.parseDebugURL", "debug URL host must be localhost or loopback", nil) + } return debugURL, nil } +func isLoopbackHost(host string) bool { + if host == "" { + return false + } + if core.Lower(host) == "localhost" { + return true + } + + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() +} + func canonicalDebugURL(debugURL *url.URL) string { return core.TrimSuffix(debugURL.String(), "/") } @@ -467,15 +487,19 @@ func doDebugRequest(ctx context.Context, debugHTTPURL *url.URL, endpoint, rawQue } defer func() { _ = resp.Body.Close() }() - r := core.ReadAll(resp.Body) - if !r.OK { - return nil, r.Value.(error) - } if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { return nil, coreerr.E("CDPClient.doDebugRequest", "debug endpoint returned "+resp.Status, nil) } - return []byte(r.Value.(string)), nil + body, err := io.ReadAll(io.LimitReader(resp.Body, maxDebugResponseBytes+1)) + if err != nil { + return nil, err + } + if len(body) > maxDebugResponseBytes { + return nil, coreerr.E("CDPClient.doDebugRequest", "debug endpoint response too large", nil) + } + + return body, nil } func listTargetsAt(ctx context.Context, debugHTTPURL *url.URL) ([]TargetInfo, error) { @@ -493,6 +517,12 @@ func listTargetsAt(ctx context.Context, debugHTTPURL *url.URL) ([]TargetInfo, er } func createTargetAt(ctx context.Context, debugHTTPURL *url.URL, pageURL string) (*TargetInfo, error) { + if pageURL != "" { + if err := validateNavigationURL(pageURL); err != nil { + return nil, coreerr.E("CDPClient.createTargetAt", "invalid page URL", err) + } + } + rawQuery := "" if pageURL != "" { rawQuery = url.QueryEscape(pageURL) @@ -525,6 +555,31 @@ func validateTargetWebSocketURL(debugHTTPURL *url.URL, raw string) (string, erro return wsURL.String(), nil } +func validateNavigationURL(raw string) error { + navigationURL, err := url.Parse(raw) + if err != nil { + return err + } + + switch core.Lower(navigationURL.Scheme) { + case "http", "https": + if navigationURL.Host == "" { + return coreerr.E("CDPClient.validateNavigationURL", "navigation URL host is required", nil) + } + if navigationURL.User != nil { + return coreerr.E("CDPClient.validateNavigationURL", "navigation URL must not include credentials", nil) + } + return nil + case "about": + if raw == "about:blank" { + return nil + } + return coreerr.E("CDPClient.validateNavigationURL", "only about:blank is permitted for non-http navigation", nil) + default: + return coreerr.E("CDPClient.validateNavigationURL", "navigation URL must use http, https, or about:blank", nil) + } +} + func sameEndpointHost(httpURL, wsURL *url.URL) bool { return core.Lower(httpURL.Hostname()) == core.Lower(wsURL.Hostname()) && normalisedPort(httpURL) == normalisedPort(wsURL) } diff --git a/console.go b/console.go index dfabd6b..6046e22 100644 --- a/console.go +++ b/console.go @@ -719,7 +719,33 @@ func FormatConsoleOutput(messages []ConsoleMessage) string { prefix = "[LOG]" } timestamp := msg.Timestamp.Format("15:04:05.000") - output.WriteString(core.Sprintf("%s %s %s\n", timestamp, prefix, msg.Text)) + output.WriteString(core.Sprintf("%s %s %s\n", timestamp, prefix, sanitizeConsoleText(msg.Text))) } return output.String() } + +func sanitizeConsoleText(text string) string { + var b strings.Builder + b.Grow(len(text)) + + for _, r := range text { + switch r { + case '\n': + b.WriteString(`\n`) + case '\r': + b.WriteString(`\r`) + case '\t': + b.WriteString(`\t`) + case '\x1b': + b.WriteString(`\x1b`) + default: + if r < 0x20 || r == 0x7f { + b.WriteByte(' ') + continue + } + b.WriteRune(r) + } + } + + return b.String() +} diff --git a/webview.go b/webview.go index f11e777..fd76674 100644 --- a/webview.go +++ b/webview.go @@ -171,11 +171,19 @@ func (wv *Webview) Navigate(url string) error { ctx, cancel := context.WithTimeout(wv.ctx, wv.timeout) defer cancel() + return wv.navigate(ctx, url, "Webview.Navigate") +} + +func (wv *Webview) navigate(ctx context.Context, rawURL, scope string) error { + if err := validateNavigationURL(rawURL); err != nil { + return coreerr.E(scope, "invalid navigation URL", err) + } + _, err := wv.client.Call(ctx, "Page.navigate", map[string]any{ - "url": url, + "url": rawURL, }) if err != nil { - return coreerr.E("Webview.Navigate", "failed to navigate", err) + return coreerr.E(scope, "failed to navigate", err) } // Wait for page load diff --git a/webview_test.go b/webview_test.go index 157422d..3022b7c 100644 --- a/webview_test.go +++ b/webview_test.go @@ -3,6 +3,10 @@ package webview import ( "context" + "io" + "net/http" + "net/http/httptest" + "strings" "testing" "time" ) @@ -417,6 +421,24 @@ func TestFormatConsoleOutput_Good_Empty(t *testing.T) { } } +// TestFormatConsoleOutput_Good_SanitisesControlCharacters verifies console output is safe for log sinks. +func TestFormatConsoleOutput_Good_SanitisesControlCharacters(t *testing.T) { + output := FormatConsoleOutput([]ConsoleMessage{ + { + Type: "error", + Text: "first line\nsecond line\x1b[31m", + Timestamp: time.Date(2026, 1, 15, 14, 30, 45, 0, time.UTC), + }, + }) + + if !containsString(output, `first line\nsecond line\x1b[31m`) { + t.Fatalf("expected control characters to be escaped, got %q", output) + } + if containsString(output, "\nsecond line") { + t.Fatalf("expected embedded newlines to be escaped, got %q", output) + } +} + // TestNormalizeConsoleType_Good verifies CDP warning aliases are normalised. func TestNormalizeConsoleType_Good(t *testing.T) { if got := normalizeConsoleType("warn"); got != "warn" { @@ -521,6 +543,82 @@ func TestFormatJSValue_Good(t *testing.T) { } } +// TestParseDebugURL_Bad_RejectsRemoteHosts verifies debug endpoints are loopback-only. +func TestParseDebugURL_Bad_RejectsRemoteHosts(t *testing.T) { + for _, raw := range []string{ + "http://example.com:9222", + "http://10.0.0.1:9222", + "http://[2001:db8::1]:9222", + } { + if _, err := parseDebugURL(raw); err == nil { + t.Fatalf("parseDebugURL(%q) returned nil error", raw) + } + } +} + +// TestParseDebugURL_Good_AllowsLoopbackHosts verifies local debugging endpoints remain usable. +func TestParseDebugURL_Good_AllowsLoopbackHosts(t *testing.T) { + for _, raw := range []string{ + "http://localhost:9222", + "http://127.0.0.1:9222", + "http://[::1]:9222", + } { + if _, err := parseDebugURL(raw); err != nil { + t.Fatalf("parseDebugURL(%q) returned error: %v", raw, err) + } + } +} + +// TestValidateNavigationURL_Good_AllowsWebURLs verifies navigation accepts HTTP(S) pages. +func TestValidateNavigationURL_Good_AllowsWebURLs(t *testing.T) { + for _, raw := range []string{ + "https://example.com", + "http://localhost:8080/path?q=1", + "about:blank", + } { + if err := validateNavigationURL(raw); err != nil { + t.Fatalf("validateNavigationURL(%q) returned error: %v", raw, err) + } + } +} + +// TestValidateNavigationURL_Bad_RejectsDangerousSchemes verifies non-web schemes are blocked. +func TestValidateNavigationURL_Bad_RejectsDangerousSchemes(t *testing.T) { + for _, raw := range []string{ + "javascript:alert(1)", + "data:text/html,hello", + "file:///etc/passwd", + "about:srcdoc", + "ftp://example.com", + } { + if err := validateNavigationURL(raw); err == nil { + t.Fatalf("validateNavigationURL(%q) returned nil error", raw) + } + } +} + +// TestDoDebugRequest_Bad_RejectsOversizedBody verifies debug responses are bounded. +func TestDoDebugRequest_Bad_RejectsOversizedBody(t *testing.T) { + var payload strings.Builder + payload.Grow(maxDebugResponseBytes + 1) + payload.WriteString(strings.Repeat("a", maxDebugResponseBytes+1)) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, payload.String()) + })) + t.Cleanup(server.Close) + + debugURL, err := parseDebugURL(server.URL) + if err != nil { + t.Fatalf("parseDebugURL returned error: %v", err) + } + + if _, err := doDebugRequest(context.Background(), debugURL, "/json", ""); err == nil { + t.Fatal("doDebugRequest returned nil error for oversized body") + } +} + // TestGetString_Good verifies map string extraction. func TestGetString_Good(t *testing.T) { m := map[string]any{