From bc52dd3067c67c83fc04a18f9db5248d223be445 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 24 May 2026 16:38:23 +0200 Subject: [PATCH] sec5c: sanitize log-injection in pkg/service/proxy and pkg/service/setup Fixes CodeQL go/log-injection alerts in the proxy and setup packages. Adds logutil.go with a package-private sanitizeLog helper to each package. pkg/service/proxy/proxy.go (2 call sites): - LogRequest: r.URL.String(), bodyStr - LogResponse: r.Request.URL.String(), bodyStr pkg/service/proxy/recorder.go (1 call site): - save: task.path (derived from external URL path segments) pkg/service/setup/setup.go (7 call sites): - SyncDeviceData: deviceIP, info.Name, info.DeviceID, info.SerialNumber - syncPresets: deviceIP - notifySpeakerSourcesUpdated: deviceIP No behaviour change. golangci-lint and make check pass. Co-Authored-By: Claude Sonnet 4.6 --- pkg/service/proxy/logutil.go | 13 +++++++++++++ pkg/service/proxy/proxy.go | 4 ++-- pkg/service/proxy/recorder.go | 2 +- pkg/service/setup/logutil.go | 13 +++++++++++++ pkg/service/setup/setup.go | 14 +++++++------- 5 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 pkg/service/proxy/logutil.go create mode 100644 pkg/service/setup/logutil.go diff --git a/pkg/service/proxy/logutil.go b/pkg/service/proxy/logutil.go new file mode 100644 index 00000000..f0213360 --- /dev/null +++ b/pkg/service/proxy/logutil.go @@ -0,0 +1,13 @@ +package proxy + +import "strings" + +// sanitizeLog strips newline characters from s to prevent log-injection +// (CodeQL go/log-injection). Values from speakers, HTTP requests, and +// external APIs may contain attacker-controlled newlines. +func sanitizeLog(s string) string { + s = strings.ReplaceAll(s, "\n", `\n`) + s = strings.ReplaceAll(s, "\r", `\r`) + + return s +} diff --git a/pkg/service/proxy/proxy.go b/pkg/service/proxy/proxy.go index 30f2d66c..ee31b0d3 100644 --- a/pkg/service/proxy/proxy.go +++ b/pkg/service/proxy/proxy.go @@ -84,7 +84,7 @@ func (lp *LoggingProxy) LogRequest(r *http.Request) { } } - log.Printf("[PROXY_REQ] %s %s\n Headers:\n%s\n Body: %s", r.Method, r.URL.String(), headers, bodyStr) + log.Printf("[PROXY_REQ] %s %s\n Headers:\n%s\n Body: %s", r.Method, sanitizeLog(r.URL.String()), headers, sanitizeLog(bodyStr)) } // LogResponse prints an abbreviated response with optional header/body redaction. @@ -108,7 +108,7 @@ func (lp *LoggingProxy) LogResponse(r *http.Response) { } } - log.Printf("[PROXY_RES] %d %s\n Headers:\n%s\n Body: %s", r.StatusCode, r.Request.URL.String(), headers, bodyStr) + log.Printf("[PROXY_RES] %d %s\n Headers:\n%s\n Body: %s", r.StatusCode, sanitizeLog(r.Request.URL.String()), headers, sanitizeLog(bodyStr)) if lp.Recorder != nil && lp.RecordEnabled { _ = lp.Recorder.Record("upstream", r.Request, r) diff --git a/pkg/service/proxy/recorder.go b/pkg/service/proxy/recorder.go index e18f7a0f..d29ea315 100644 --- a/pkg/service/proxy/recorder.go +++ b/pkg/service/proxy/recorder.go @@ -400,7 +400,7 @@ func (r *Recorder) save(task recordingTask) { } if err := r.rootWriteFile(task.path, buf.Bytes(), 0644); err != nil { - log.Printf("failed to write recording to %s: %v", task.path, err) + log.Printf("failed to write recording to %s: %v", sanitizeLog(task.path), err) } _ = r.updateEnvFile(task.replacements) diff --git a/pkg/service/setup/logutil.go b/pkg/service/setup/logutil.go new file mode 100644 index 00000000..c6c5dc5c --- /dev/null +++ b/pkg/service/setup/logutil.go @@ -0,0 +1,13 @@ +package setup + +import "strings" + +// sanitizeLog strips newline characters from s to prevent log-injection +// (CodeQL go/log-injection). Values from speakers, HTTP requests, and +// external APIs may contain attacker-controlled newlines. +func sanitizeLog(s string) string { + s = strings.ReplaceAll(s, "\n", `\n`) + s = strings.ReplaceAll(s, "\r", `\r`) + + return s +} diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 7ff7dc3c..14c284fd 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -2499,18 +2499,18 @@ func (m *Manager) SyncDeviceData(deviceIP string) error { } log.Printf("Starting sync for device at %s: Name='%s', DeviceID='%s', SerialNumber='%s'", - deviceIP, info.Name, info.DeviceID, info.SerialNumber) + sanitizeLog(deviceIP), sanitizeLog(info.Name), sanitizeLog(info.DeviceID), sanitizeLog(info.SerialNumber)) accountID := "" // Use deviceID from /info as canonical identifier (MAC address) deviceID := info.DeviceID if deviceID == "" { - log.Printf("No deviceID found in /info response for device '%s' at %s", info.Name, deviceIP) + log.Printf("No deviceID found in /info response for device '%s' at %s", sanitizeLog(info.Name), sanitizeLog(deviceIP)) return fmt.Errorf("no deviceID found in /info response for device at %s - cannot sync without canonical device identifier", deviceIP) } - log.Printf("Using deviceID '%s' for sync operations (MAC address from /info)", deviceID) + log.Printf("Using deviceID '%s' for sync operations (MAC address from /info)", sanitizeLog(deviceID)) if info.MargeAccountUUID != "" { accountID = info.MargeAccountUUID @@ -2561,11 +2561,11 @@ func (m *Manager) syncPresets(deviceIP, accountID, deviceID string) { presetsURL = fmt.Sprintf("http://%s/presets", deviceIP) } - log.Printf("[SYNC] Syncing presets for %s", deviceIP) + log.Printf("[SYNC] Syncing presets for %s", sanitizeLog(deviceIP)) resp, err := m.HTTPGet(presetsURL) if err != nil { - log.Printf("[SYNC_ERR] Failed to fetch presets for %s: %v", deviceIP, err) + log.Printf("[SYNC_ERR] Failed to fetch presets for %s: %v", sanitizeLog(deviceIP), err) return } @@ -2754,9 +2754,9 @@ func (m *Manager) syncSources(deviceIP, accountID, deviceID string) { func (m *Manager) notifySpeakerSourcesUpdated(deviceIP, deviceID string) { c := client.NewClientFromHost(deviceIP) if err := c.NotifySourcesUpdated(deviceID); err != nil { - log.Printf("[SYNC] notify %s: %v", deviceIP, err) + log.Printf("[SYNC] notify %s: %v", sanitizeLog(deviceIP), err) return } - log.Printf("[SYNC] notify %s sourcesUpdated -> ok", deviceIP) + log.Printf("[SYNC] notify %s sourcesUpdated -> ok", sanitizeLog(deviceIP)) }