diff --git a/html/js/app.js b/html/js/app.js index 7e2a379..7750cb8 100644 --- a/html/js/app.js +++ b/html/js/app.js @@ -7,7 +7,6 @@ class Server { return; } SERVER_CONNECTION = true; - console.log(data); if (this.cmd != "updateLog") { showElement("loading", true); UNDO = new Object(); @@ -25,7 +24,8 @@ class Server { if (wsHost == undefined || wsHost.length < 1) { wsHost = window.location.hostname; } - var url = this.protocol + wsHost + "/data/" + "?Token=" + getCookie("Token"); + // The session cookie (HttpOnly) is sent with the websocket handshake. + var url = this.protocol + wsHost + "/data/"; data["cmd"] = this.cmd; var requestCmd = data["cmd"]; var ws = new WebSocket(url); @@ -75,10 +75,6 @@ class Server { if (data["cmd"] != "updateLog") { setConnectionState("busy"); } - console.log("REQUEST (JS):"); - console.log(data); - console.log("REQUEST: (JSON)"); - console.log(JSON.stringify(data)); this.send(JSON.stringify(data)); }; ws.onerror = function (e) { @@ -95,12 +91,7 @@ class Server { ws.onmessage = function (e) { responseReceived = true; finishRequest("online", true); - console.log("RESPONSE:"); var response = JSON.parse(e.data); - console.log(response); - if (response.hasOwnProperty("token")) { - document.cookie = "Token=" + response["token"]; - } if (response["status"] == false) { setConnectionState("offline"); alert(response["err"]); @@ -162,12 +153,6 @@ class Server { } } var WS_FAILURE_COUNT = 0; -function getCookie(name) { - var value = "; " + document.cookie; - var parts = value.split("; " + name + "="); - if (parts.length == 2) - return parts.pop().split(";").shift(); -} class MainMenu { constructor() { this.DocumentID = "main-menu"; @@ -181,7 +166,7 @@ class MainMenu { } createValue(value) { var element = document.createElement("P"); - element.innerHTML = value; + element.textContent = value; return element; } } @@ -233,7 +218,7 @@ class Content { } createHeadline(value) { var element = document.createElement("H3"); - element.innerHTML = value; + element.textContent = value; return element; } createHR() { @@ -584,7 +569,7 @@ class Cell { switch (this.childType) { case "P": element = document.createElement(this.childType); - element.innerHTML = this.value; + element.textContent = this.value; element.className = this.className; break; case "INPUT": @@ -632,7 +617,7 @@ class Cell { td.appendChild(element); } else { - td.innerHTML = this.value; + td.textContent = this.value; } if (this.onclick == true) { td.setAttribute("onclick", this.onclickFunktion); @@ -1022,7 +1007,7 @@ function setClientInfoValue(key, value) { element.appendChild(anchor); return; } - element.innerHTML = textValue; + element.textContent = textValue; } function createLayout() { // Client Info @@ -1168,11 +1153,17 @@ class PopupContent extends PopupWindow { s.options[s.selectedIndex].value = value; return select; } - description(value) { + // isHTML must only be true for static text written in the source; never for data. + description(value, isHTML = false) { var tr = document.createElement("TR"); var td = document.createElement("TD"); var span = document.createElement("PRE"); - span.innerHTML = value; + if (isHTML) { + span.innerHTML = value; + } + else { + span.textContent = value; + } tr.appendChild(td); tr.appendChild(this.createContent(span)); this.table.appendChild(tr); @@ -1278,7 +1269,7 @@ function openPopUp(dataType, element) { input.className = "notAvailable"; content.appendRow("Tuner / Streams", input); } - content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams."); + content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams.", true); // Interaktion content.createInteraction(); // Löschen @@ -1348,7 +1339,7 @@ function openPopUp(dataType, element) { input.className = "notAvailable"; content.appendRow("Tuner / Streams", input); } - content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams."); + content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams.", true); // Interaktion content.createInteraction(); // Löschen @@ -1448,7 +1439,7 @@ function openPopUp(dataType, element) { var select = content.createSelect(text, values, data[dbKey], dbKey); select.setAttribute("onchange", "javascript: this.className = 'changed'"); content.appendRow("Group Title", select); - content.description("Select a M3U group. (Counter)
Changing the group title in the M3U invalidates the filter."); + content.description("Select a M3U group. (Counter)
Changing the group title in the M3U invalidates the filter.", true); // Groß- Kleinschreibung beachten var dbKey = "caseSensitive"; var input = content.createCheckbox(dbKey); @@ -1458,12 +1449,12 @@ function openPopUp(dataType, element) { var input = content.createInput("text", dbKey, data[dbKey]); input.setAttribute("placeholder", "FHD,UHD"); content.appendRow("Include", input); - content.description("Channel name must include.
(Comma separated) Comma means or"); + content.description("Channel name must include.
(Comma separated) Comma means or", true); var dbKey = "exclude"; var input = content.createInput("text", dbKey, data[dbKey]); input.setAttribute("placeholder", "ES,IT"); content.appendRow("Exclude", input); - content.description("Channel name must not contain.
(Comma separated) Comma means or"); + content.description("Channel name must not contain.
(Comma separated) Comma means or", true); break; default: break; @@ -2030,13 +2021,13 @@ function donePopupData(dataType, idsStr) { //(document.getElementById(id).childNodes[2].firstChild as HTMLElement).setAttribute("src", value) break; case "x-name": - document.getElementById(id).childNodes[3].firstChild.innerHTML = value; + document.getElementById(id).childNodes[3].firstChild.textContent = value; break; case "x-category": document.getElementById(id).childNodes[3].firstChild.className = value; break; case "x-group-title": - document.getElementById(id).childNodes[5].firstChild.innerHTML = value; + document.getElementById(id).childNodes[5].firstChild.textContent = value; break; case "x-xmltv-file": if (value != "xTeVe Dummy" && value != "-") { @@ -2045,13 +2036,13 @@ function donePopupData(dataType, idsStr) { if (value == "-") { input["x-active"] = false; } - document.getElementById(id).childNodes[6].firstChild.innerHTML = value; + document.getElementById(id).childNodes[6].firstChild.textContent = value; break; case "x-mapping": if (value == "-") { input["x-active"] = false; } - document.getElementById(id).childNodes[7].firstChild.innerHTML = value; + document.getElementById(id).childNodes[7].firstChild.textContent = value; break; default: } @@ -2658,7 +2649,7 @@ class Log { if (entry.indexOf("DEBUG") != -1) { element.className = "debugMsg"; } - element.innerHTML = entry; + element.textContent = entry; return element; } } diff --git a/src/internal/authentication/authentication.go b/src/internal/authentication/authentication.go index 0fc0870..50f1a90 100755 --- a/src/internal/authentication/authentication.go +++ b/src/internal/authentication/authentication.go @@ -503,9 +503,21 @@ loopToken: } // SetCookieToken : set cookie +// SetCookieToken : sets the session cookie. It is HttpOnly (scripts cannot +// read it), SameSite=Strict, and a session cookie: expiry is enforced server +// side per token and refreshed on every authenticated request. A token of +// "-" clears the cookie (logout). func SetCookieToken(w http.ResponseWriter, token string) http.ResponseWriter { - expiration := time.Now().Add(time.Minute * time.Duration(tokenValidity)) - cookie := http.Cookie{Name: "Token", Value: token, Expires: expiration} + cookie := http.Cookie{ + Name: "Token", + Value: token, + Path: "/", + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + } + if token == "-" { + cookie.MaxAge = -1 + } http.SetCookie(w, &cookie) return w } diff --git a/src/webserver.go b/src/webserver.go index 15a467b..be825e6 100644 --- a/src/webserver.go +++ b/src/webserver.go @@ -15,12 +15,29 @@ import ( "github.com/gorilla/websocket" ) +// wsUpgrader uses gorilla's default CheckOrigin: an Origin header, when +// present, must match the request Host. That blocks cross-site websocket +// hijacking from other pages on the LAN while still allowing non-browser +// clients that send no Origin. var wsUpgrader = websocket.Upgrader{ ReadBufferSize: 4096, WriteBufferSize: 4096, - CheckOrigin: func(r *http.Request) bool { - return true - }, +} + +// wsSessionToken : session token for a websocket request. The HttpOnly +// cookie set at login is preferred; the legacy ?Token= query parameter is +// still accepted for older clients. +func wsSessionToken(r *http.Request) string { + + if c, err := r.Cookie("Token"); err == nil && len(c.Value) > 0 { + return c.Value + } + + if v := r.URL.Query().Get("Token"); len(v) > 0 { + return v + } + + return "-" } // StartWebserver : Startet den Webserver @@ -313,10 +330,10 @@ func WS(w http.ResponseWriter, r *http.Request) { } */ + // Upgrade writes its own error response (e.g. 403 for a bad Origin). conn, err := wsUpgrader.Upgrade(w, r, nil) if err != nil { ShowError(err, 0) - http.Error(w, "Could not open websocket connection", http.StatusBadRequest) return } @@ -337,16 +354,7 @@ func WS(w http.ResponseWriter, r *http.Request) { // Token Authentication case true: - var token string - tokens, ok := r.URL.Query()["Token"] - - if !ok || len(tokens[0]) < 1 { - token = "-" - } else { - token = tokens[0] - } - - newToken, err = tokenAuthentication(token) + newToken, err = tokenAuthentication(wsSessionToken(r)) if err != nil { response.Status = false @@ -602,22 +610,21 @@ func Web(w http.ResponseWriter, r *http.Request) { } - switch System.ConfigurationWizard { - - case true: - file = "configuration.html" - Settings.AuthenticationWEB = false - - case false: - file = "index.html" - - } + file = "index.html" if System.ScanInProgress == 1 { file = "maintenance.html" } - switch Settings.AuthenticationWEB { + // The first-run wizard is reachable without a login (there is nothing + // to protect yet); it must not switch authentication off in Settings. + var requireLogin = Settings.AuthenticationWEB && !System.ConfigurationWizard + + if System.ConfigurationWizard { + file = "configuration.html" + } + + switch requireLogin { case true: var username, password, confirm string diff --git a/src/websocket_test.go b/src/websocket_test.go new file mode 100644 index 0000000..611b01a --- /dev/null +++ b/src/websocket_test.go @@ -0,0 +1,139 @@ +package src + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gorilla/websocket" + + "xteve/src/internal/authentication" +) + +// newAuthenticatedWSServer starts the websocket handler with web +// authentication enabled and returns the server plus a valid session token. +func newAuthenticatedWSServer(t *testing.T) (*httptest.Server, string) { + t.Helper() + + if err := authentication.Init(t.TempDir()+"/authentication.json", 60); err != nil { + t.Fatalf("authentication.Init: %v", err) + } + if _, err := authentication.CreateNewUser("admin", "secret"); err != nil { + t.Fatalf("CreateNewUser: %v", err) + } + token, err := authentication.UserAuthentication("admin", "secret") + if err != nil { + t.Fatalf("UserAuthentication: %v", err) + } + + Settings.AuthenticationWEB = true + System.ConfigurationWizard = false + t.Cleanup(func() { Settings.AuthenticationWEB = false }) + + srv := httptest.NewServer(http.HandlerFunc(WS)) + t.Cleanup(srv.Close) + return srv, token +} + +func wsURL(srv *httptest.Server) string { + return "ws" + strings.TrimPrefix(srv.URL, "http") + "/data/" +} + +// roundTrip sends one command and returns the parsed response. +func roundTrip(t *testing.T, url string, header http.Header) ResponseStruct { + t.Helper() + + conn, resp, err := websocket.DefaultDialer.Dial(url, header) + if err != nil { + t.Fatalf("dial: %v (resp=%v)", err, resp) + } + defer conn.Close() + + if err := conn.WriteJSON(map[string]any{"cmd": "noop"}); err != nil { + t.Fatalf("write: %v", err) + } + var response ResponseStruct + if err := conn.ReadJSON(&response); err != nil { + t.Fatalf("read: %v", err) + } + return response +} + +func TestWSRejectsCrossOrigin(t *testing.T) { + srv, _ := newAuthenticatedWSServer(t) + + header := http.Header{"Origin": {"http://evil.example"}} + _, resp, err := websocket.DefaultDialer.Dial(wsURL(srv), header) + if err == nil { + t.Fatal("expected the cross-origin upgrade to be refused") + } + if resp == nil || resp.StatusCode != http.StatusForbidden { + t.Fatalf("expected 403, got %v", resp) + } +} + +func TestWSAllowsSameOriginAndNoOrigin(t *testing.T) { + srv, token := newAuthenticatedWSServer(t) + + sameOrigin := http.Header{ + "Origin": {srv.URL}, + "Cookie": {"Token=" + token}, + } + if r := roundTrip(t, wsURL(srv), sameOrigin); !r.Status { + t.Fatalf("same-origin request with cookie should succeed, got error %q", r.Error) + } + + noOrigin := http.Header{"Cookie": {"Token=" + token}} + if r := roundTrip(t, wsURL(srv), noOrigin); !r.Status { + t.Fatalf("request without Origin (non-browser client) should succeed, got error %q", r.Error) + } +} + +func TestWSRequiresSessionToken(t *testing.T) { + srv, token := newAuthenticatedWSServer(t) + + r := roundTrip(t, wsURL(srv), nil) + if r.Status || !r.Reload { + t.Fatalf("request without a token should be refused with reload, got status=%v reload=%v", r.Status, r.Reload) + } + + bad := http.Header{"Cookie": {"Token=not-a-real-token"}} + if r := roundTrip(t, wsURL(srv), bad); r.Status { + t.Fatal("request with an unknown token should be refused") + } + + // Legacy clients may still pass the token as a query parameter. + if r := roundTrip(t, wsURL(srv)+"?Token="+token, nil); !r.Status { + t.Fatalf("legacy query-parameter token should be accepted, got error %q", r.Error) + } +} + +func TestSetCookieTokenFlags(t *testing.T) { + rec := httptest.NewRecorder() + authentication.SetCookieToken(rec, "abc") + + cookies := rec.Result().Cookies() + if len(cookies) != 1 { + t.Fatalf("expected one cookie, got %d", len(cookies)) + } + c := cookies[0] + if c.Name != "Token" || c.Value != "abc" { + t.Fatalf("unexpected cookie %v", c) + } + if !c.HttpOnly { + t.Error("session cookie must be HttpOnly") + } + if c.SameSite != http.SameSiteStrictMode { + t.Error("session cookie must be SameSite=Strict") + } + if c.Path != "/" { + t.Errorf("cookie path should be /, got %q", c.Path) + } + + rec = httptest.NewRecorder() + authentication.SetCookieToken(rec, "-") + if c := rec.Result().Cookies()[0]; c.MaxAge >= 0 { + t.Error("logout must clear the cookie (negative MaxAge)") + } +} diff --git a/tasks/todo.md b/tasks/todo.md index e301ee0..9af4503 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -13,10 +13,10 @@ Status: Phase 0 committed on branch `improvements` 2026-09-26. - [x] staticcheck baseline via `go run honnef.co/go/tools/cmd/staticcheck@latest ./...`: 408 findings (S1002 113, SA5008 79, S1039 67, S1038 40, S1023 35, ST1005 20, SA1019 14, SA4006 9, misc 11). Mostly style; SA5008/SA4006/SA1019 worth a pass in Phase 3 cleanup ## Phase 1: Security -- [ ] Replace `innerHTML` with `textContent` for provider-controlled strings -- [ ] Enforce websocket Origin check; move token from query string to cookie; stop console-logging tokens -- [ ] Cookie `HttpOnly` + `SameSite` -- [ ] Wizard GET must not set `AuthenticationWEB = false` +- [x] `textContent` for provider-controlled strings (cells, client info, in-place edits, logs, popup descriptions with an explicit static-HTML flag) +- [x] Websocket uses gorilla's same-origin check; token read from the HttpOnly cookie (query param kept for legacy clients); payload/token console logging removed. Tests in `src/websocket_test.go` +- [x] Cookie `HttpOnly`, `SameSite=Strict`, `Path=/`, cleared on logout +- [x] Wizard GET no longer mutates `AuthenticationWEB`; the wizard page just bypasses login while active - [ ] Sanitise `uploadLogo` filename - [ ] Zip-slip guard in backup restore - [ ] Restrict `ffmpeg.path` / `vlc.path` and reject non-http(s) stream URLs diff --git a/ts/logs_ts.ts b/ts/logs_ts.ts index 6b40916..2391f70 100644 --- a/ts/logs_ts.ts +++ b/ts/logs_ts.ts @@ -16,7 +16,7 @@ class Log { element.className = "debugMsg" } - element.innerHTML = entry + element.textContent = entry return element } diff --git a/ts/menu_ts.ts b/ts/menu_ts.ts index 346ffc9..54f5d96 100644 --- a/ts/menu_ts.ts +++ b/ts/menu_ts.ts @@ -12,7 +12,7 @@ class MainMenu { createValue(value):any { var element = document.createElement("P") - element.innerHTML = value + element.textContent = value return element } } @@ -85,7 +85,7 @@ class Content { createHeadline(value):any { var element = document.createElement("H3") - element.innerHTML = value + element.textContent = value return element } @@ -531,7 +531,7 @@ class Cell { switch(this.childType){ case "P": element = document.createElement(this.childType); - element.innerHTML = this.value + element.textContent = this.value element.className = this.className break @@ -586,7 +586,7 @@ class Cell { td.appendChild(element) } else { - td.innerHTML = this.value + td.textContent = this.value } if (this.onclick == true) { @@ -1081,7 +1081,7 @@ function setClientInfoValue(key:string, value:any) { return } - element.innerHTML = textValue + element.textContent = textValue } function createLayout() { @@ -1266,12 +1266,17 @@ class PopupContent extends PopupWindow{ return select } - description(value:string):any { + // isHTML must only be true for static text written in the source; never for data. + description(value:string, isHTML:boolean = false):any { var tr = document.createElement("TR") var td = document.createElement("TD") var span = document.createElement("PRE") - span.innerHTML = value + if (isHTML) { + span.innerHTML = value + } else { + span.textContent = value + } tr.appendChild(td) @@ -1398,7 +1403,7 @@ function openPopUp(dataType, element) { content.appendRow("Tuner / Streams", input) } - content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams.") + content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams.", true) // Interaktion content.createInteraction() @@ -1477,7 +1482,7 @@ function openPopUp(dataType, element) { content.appendRow("Tuner / Streams", input) } - content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams.") + content.description("Number of parallel connections that can be established to the provider.
Only available with activated buffer.
New settings will only be applied after quitting all streams.", true) // Interaktion content.createInteraction() @@ -1597,7 +1602,7 @@ function openPopUp(dataType, element) { var select = content.createSelect(text, values, data[dbKey], dbKey) select.setAttribute("onchange", "javascript: this.className = 'changed'") content.appendRow("Group Title", select) - content.description("Select a M3U group. (Counter)
Changing the group title in the M3U invalidates the filter.") + content.description("Select a M3U group. (Counter)
Changing the group title in the M3U invalidates the filter.", true) // Groß- Kleinschreibung beachten var dbKey:string = "caseSensitive" @@ -1611,13 +1616,13 @@ function openPopUp(dataType, element) { input.setAttribute("placeholder", "FHD,UHD") content.appendRow("Include", input) - content.description("Channel name must include.
(Comma separated) Comma means or") + content.description("Channel name must include.
(Comma separated) Comma means or", true) var dbKey:string = "exclude" var input = content.createInput("text", dbKey, data[dbKey]) input.setAttribute("placeholder", "ES,IT") content.appendRow("Exclude", input) - content.description("Channel name must not contain.
(Comma separated) Comma means or") + content.description("Channel name must not contain.
(Comma separated) Comma means or", true) break @@ -2359,7 +2364,7 @@ function donePopupData(dataType:string, idsStr:string) { break case "x-name": - (document.getElementById(id).childNodes[3].firstChild as HTMLElement).innerHTML = value + (document.getElementById(id).childNodes[3].firstChild as HTMLElement).textContent = value break case "x-category": @@ -2367,7 +2372,7 @@ function donePopupData(dataType:string, idsStr:string) { break case "x-group-title": - (document.getElementById(id).childNodes[5].firstChild as HTMLElement).innerHTML = value + (document.getElementById(id).childNodes[5].firstChild as HTMLElement).textContent = value break case "x-xmltv-file": @@ -2379,7 +2384,7 @@ function donePopupData(dataType:string, idsStr:string) { input["x-active"] = false } - (document.getElementById(id).childNodes[6].firstChild as HTMLElement).innerHTML = value + (document.getElementById(id).childNodes[6].firstChild as HTMLElement).textContent = value break case "x-mapping": @@ -2387,7 +2392,7 @@ function donePopupData(dataType:string, idsStr:string) { input["x-active"] = false } - (document.getElementById(id).childNodes[7].firstChild as HTMLElement).innerHTML = value + (document.getElementById(id).childNodes[7].firstChild as HTMLElement).textContent = value break diff --git a/ts/network_ts.ts b/ts/network_ts.ts index a292955..dac98ba 100644 --- a/ts/network_ts.ts +++ b/ts/network_ts.ts @@ -14,7 +14,6 @@ class Server { SERVER_CONNECTION = true - console.log(data) if (this.cmd != "updateLog") { showElement("loading", true) UNDO = new Object() @@ -34,7 +33,8 @@ class Server { if (wsHost == undefined || wsHost.length < 1) { wsHost = window.location.hostname } - var url = this.protocol + wsHost + "/data/" + "?Token=" + getCookie("Token") + // The session cookie (HttpOnly) is sent with the websocket handshake. + var url = this.protocol + wsHost + "/data/" data["cmd"] = this.cmd var requestCmd:string = data["cmd"] @@ -92,12 +92,6 @@ class Server { setConnectionState("busy") } - console.log("REQUEST (JS):"); - console.log(data) - - console.log("REQUEST: (JSON)"); - console.log(JSON.stringify(data)) - this.send(JSON.stringify(data)); } @@ -122,14 +116,7 @@ class Server { responseReceived = true finishRequest("online", true) - console.log("RESPONSE:"); var response = JSON.parse(e.data); - - console.log(response); - - if (response.hasOwnProperty("token")) { - document.cookie = "Token=" + response["token"] - } if (response["status"] == false) { setConnectionState("offline") @@ -213,8 +200,3 @@ class Server { var WS_FAILURE_COUNT:number = 0 -function getCookie(name) { - var value = "; " + document.cookie; - var parts = value.split("; " + name + "="); - if (parts.length == 2) return parts.pop().split(";").shift(); -}