github/Bose-SoundTouch
1
0
Fork 0
mirror of https://github.com/gesellix/Bose-SoundTouch.git synced 2026-08-20 09:36:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): address CodeQL findings on Stockholm + SiriusXM stubs

Browse Source 
Two of the eight CodeQL alerts on PR #313 had clean, low-cost fixes:

  - go/clear-text-logging (#141, #142): the SiriusXM stub logged the
    raw Authorization header value at INFO. The header carries a
    long-lived bearer token (margeAuthToken) — capturing service logs
    would yield replayable credentials. Switch to logging only the
    boolean presence (`authPresent=%t`).

  - go/bad-redirect-check (#138): the Stockholm handler's bare-path
    redirect uses cfg.BasePath verbatim. basePath is operator-provided
    (CLI flag / STOCKHOLM_BASE_PATH env), not request input — but a
    value like "//evil.com" would still produce a scheme-relative
    redirect to an external host. Reject any leading-double-slash or
    embedded backslash at construction time so the redirect target
    can only ever be an absolute local path.

The remaining CodeQL alerts are out of scope here:

  - go/request-forgery on proxy.go (#139, #140): the /api/http-proxy
    endpoint takes a user-provided url= parameter and fetches it by
    design — that's the whole point of the proxy. Mitigations
    already in place: isProxyLoop rejects self-references; the proxy
    is only reachable under a LAN trust model.

  - go/path-injection on static.go (#143, #144, #145): the
    path-traversal guard in resolveStaticFile (string-prefix check
    on absolute paths) is sound, but CodeQL doesn't trace it across
    the function boundary. A clearer refactor to filepath.Rel might
    silence the alert; deferred.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-17 15:05:39 +02:00
co-authored by Claude Opus 4.7
parent a7f90f4151
commit 4507d82b4c
2 changed files with 22 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/service/handlers/handlers_bmx_siriusxm.go
+9 -4
View File
@@ -21,10 +21,15 @@ import (
// HandleSiriusXMLiveAdapter returns the SIRIUSXM_EVEREST service descriptor
// from bmx_services.json for the bare live-adapter base URL.
//
// NB: we log the *presence* of the Authorization header, not its value —
// the header carries a long-lived bearer token (margeAuthToken) that
// would be replayable if a logfile got captured. CodeQL
// go/clear-text-logging caught the original `auth=%q` shape.
func (s *Server) HandleSiriusXMLiveAdapter(w http.ResponseWriter, r *http.Request) {
log.Printf("[BMX SiriusXM] %s %s ua=%q auth=%q query=%q",
log.Printf("[BMX SiriusXM] %s %s ua=%q authPresent=%t query=%q",
r.Method, r.URL.Path, r.UserAgent(),
r.Header.Get("Authorization"), r.URL.RawQuery)
r.Header.Get("Authorization") != "", r.URL.RawQuery)
svc, err := extractBMXService(bmxServicesJSON, "SIRIUSXM_EVEREST")
if err != nil {
@@ -45,9 +50,9 @@ func (s *Server) HandleSiriusXMLiveAdapter(w http.ResponseWriter, r *http.Reques
// pass — the _links in the descriptor publish /availability, /token,
// /navigate, /logout; playback URLs come dynamically from navigate.
func (s *Server) HandleSiriusXMLiveAdapterSubpath(w http.ResponseWriter, r *http.Request) {
log.Printf("[BMX SiriusXM] UNIMPLEMENTED %s %s ua=%q auth=%q query=%q",
log.Printf("[BMX SiriusXM] UNIMPLEMENTED %s %s ua=%q authPresent=%t query=%q",
r.Method, r.URL.Path, r.UserAgent(),
r.Header.Get("Authorization"), r.URL.RawQuery)
r.Header.Get("Authorization") != "", r.URL.RawQuery)
http.Error(w, "not implemented", http.StatusNotFound)
}
pkg/service/stockholm/handler.go
+13
View File
@@ -64,6 +64,19 @@ func New(stockholmDir, workspaceRoot, backendURL, basePath string) (*Handler, er
}
basePath = strings.TrimRight(basePath, "/")
// Defence in depth: basePath is operator-provided (CLI flag /
// STOCKHOLM_BASE_PATH env var), not request input — but if it
// were ever set to "//evil.com" (typo or hostile env injection)
// the bare-path redirect below would go scheme-relative to
// evil.com. Reject any leading-double-slash and any backslash
// so the redirect target can only ever be an absolute local
// path. CodeQL go/bad-redirect-check raised the original
// concern.
if strings.HasPrefix(basePath, "//") || strings.HasPrefix(basePath, "/\\") || strings.ContainsAny(basePath, "\\") {
return nil, fmt.Errorf("invalid stockholm base path %q: must be an absolute path starting with a single '/'", basePath)
}
cfg.BasePath = basePath
state.SeedFromEnv(cfg)