From ddd857173b0771e38abba8e58f8f89894d6fc506 Mon Sep 17 00:00:00 2001 From: Andreas Wrede Date: Mon, 8 Jun 2026 13:06:05 -0400 Subject: [PATCH] fix: address security vulnerabilities from audit - Path traversal: confine avatar file serving to avatar_dir (defaults to config file directory); validate on both read and write - UDP owner injection: server-configured owner now takes precedence over UDP-supplied value, matching the documented intent - Open redirect: reject non-relative next= values after login - Stored XSS: enable Jinja2 autoescape on all template environments; add escHtml() helper in live.html and apply to all innerHTML sinks sourced from network data (host names, addrs, states, log messages) Co-Authored-By: Claude Sonnet 4.6 --- hbd/server/http.py | 32 +++++++++++++++++++++++++------- hbd/server/templates/live.html | 30 ++++++++++++++++++------------ hbd/server/udp.py | 2 +- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/hbd/server/http.py b/hbd/server/http.py index b7fc838..b86d945 100644 --- a/hbd/server/http.py +++ b/hbd/server/http.py @@ -424,7 +424,7 @@ async def start( # Resolve templates directory relative to the hbd package pkg_dir = os.path.dirname(__file__) templates_dir = config.get("templates_dir", os.path.join(pkg_dir, "templates")) - env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir)) + env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir), autoescape=True) host = config.get("hb_host", "localhost") extra_scripts = config.get("http_extra_scripts", "") host = request.host # includes port if non-standard @@ -690,7 +690,7 @@ async def start( current_user, _ = _require_auth_redirect(request) pkg_dir = os.path.dirname(__file__) templates_dir = config.get("templates_dir", os.path.join(pkg_dir, "templates")) - env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir)) + env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir), autoescape=True) # Collect all hosts with plugin data (filtered by visibility) hosts_with_plugins = [] @@ -721,7 +721,7 @@ async def start( current_user, _ = _require_auth_redirect(request) pkg_dir = os.path.dirname(__file__) templates_dir = config.get("templates_dir", os.path.join(pkg_dir, "templates")) - env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir)) + env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir), autoescape=True) tmpl = env.get_template("alerts.html") body = tmpl.render( @@ -778,6 +778,8 @@ async def start( token = users_mod.create_session(username) eventlog("hbd", "INFO", f"Login: {username} via password") redirect_to = request.rel_url.query.get("next", "/") + if not redirect_to.startswith("/"): + redirect_to = "/" resp = web.HTTPFound(redirect_to) resp.set_cookie( SESSION_COOKIE, @@ -889,6 +891,13 @@ async def start( if not target_user.avatar_is_local(): return web.Response(status=404, text="No local avatar configured") path = target_user.avatar + avatar_dir = config.get("avatar_dir") or ( + os.path.dirname(os.path.realpath(_config_path)) if _config_path else None + ) + if not avatar_dir: + return web.Response(status=403, text="Local avatars not configured") + if not os.path.realpath(path).startswith(os.path.realpath(avatar_dir) + os.sep): + return web.Response(status=403, text="Forbidden") if not os.path.isfile(path): return web.Response(status=404, text="Avatar file not found") # Infer content-type from extension @@ -992,7 +1001,7 @@ async def start( current_user, _ = _require_auth_redirect(request) pkg_dir = os.path.dirname(__file__) templates_dir = config.get("templates_dir", os.path.join(pkg_dir, "templates")) - env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir)) + env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir), autoescape=True) # Build host access summary for this user. # Merge live hosts with config-only hosts (not yet seen) so the profile @@ -1076,7 +1085,7 @@ async def start( current_user, _ = _require_auth_redirect(request) pkg_dir = os.path.dirname(__file__) templates_dir = config.get("templates_dir", os.path.join(pkg_dir, "templates")) - env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir)) + env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir), autoescape=True) from hbd import __version__ as hbd_version uptime_secs = int(time.time() - _start_epoch) @@ -1120,7 +1129,7 @@ async def start( raise web.HTTPForbidden(reason="Admin access required") pkg_dir = os.path.dirname(__file__) templates_dir = config.get("templates_dir", os.path.join(pkg_dir, "templates")) - env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir)) + env = jinja2.Environment(loader=jinja2.FileSystemLoader(templates_dir), autoescape=True) tmpl = env.get_template("settings.html") settings_data = settings_mod.get_settings_data(config, threshold_checker=threshold_checker) body = tmpl.render( @@ -1659,7 +1668,16 @@ async def start( if "full_name" in body: user_entry["full_name"] = str(body["full_name"]) if "avatar" in body: - user_entry["avatar"] = str(body["avatar"]) + avatar_val = str(body["avatar"]) + if avatar_val.startswith("/"): + avatar_dir = config.get("avatar_dir") or ( + os.path.dirname(os.path.realpath(_config_path)) if _config_path else None + ) + if not avatar_dir: + return web.json_response({"error": "Local avatars not configured"}, status=400) + if not os.path.realpath(avatar_val).startswith(os.path.realpath(avatar_dir) + os.sep): + return web.json_response({"error": "Avatar path outside allowed directory"}, status=400) + user_entry["avatar"] = avatar_val if "notification_channels" in body: visible = _visible_channels_for_user(user) user_entry["notification_channels"] = [ diff --git a/hbd/server/templates/live.html b/hbd/server/templates/live.html index 99a03de..2f2c2b8 100644 --- a/hbd/server/templates/live.html +++ b/hbd/server/templates/live.html @@ -321,9 +321,15 @@ var c = 0; var HBD_VERSION = "{{ hbd_version }}"; + function escHtml(s) { + var d = document.createElement('div'); + d.textContent = String(s); + return d.innerHTML; + } + function hostNameHtml(data) { var rawName = data.raw_name || data.name.replace(/<[^>]+>/g, '').replace('*', '').trim(); - var nameHtml = data.name; + var nameHtml = escHtml(data.name); if (!data.hbc_version || data.hbc_version !== HBD_VERSION) { nameHtml += ' 🥀'; } @@ -410,11 +416,11 @@ c_critical.innerHTML = ""; } - c_ipv4addr.innerHTML = data.connections[0].addr; - c_ipv4state.innerHTML = data.connections[0].state; + c_ipv4addr.innerHTML = escHtml(data.connections[0].addr); + c_ipv4state.innerHTML = escHtml(data.connections[0].state); if (data.connections.length > 1) { - c_ipv6addr.innerHTML = data.connections[1].addr; - c_ipv6state.innerHTML = data.connections[1].state; + c_ipv6addr.innerHTML = escHtml(data.connections[1].addr); + c_ipv6state.innerHTML = escHtml(data.connections[1].state); } var table = document.getElementById("ntablebody"); // find table to append to table.appendChild(row); // append row to table @@ -477,7 +483,7 @@ for (var i = 0; i < data.connections.length; i++) { // Offset by 2 for the warning/critical count columns - name_idx[data.name].cells[3 + i * 4].innerHTML = data.connections[i].addr; + name_idx[data.name].cells[3 + i * 4].innerHTML = escHtml(data.connections[i].addr); name_idx[data.name].cells[6 + i * 4].innerHTML = formatTS( data.connections[i].statetime ); @@ -497,7 +503,7 @@ state = 'overdue'; latency = "-"; } else { - state = "" + data.connections[i].state + ""; + state = "" + escHtml(data.connections[i].state) + ""; latency = "-"; } } @@ -558,12 +564,12 @@ + ' ' + _p(_d.getHours()) + ':' + _p(_d.getMinutes()) + ':' + _p(_d.getSeconds()); var lvl = (msg.level || "INFO").toLowerCase(); var hostVal = msg.host || ''; - var html = '
'; + var html = '
'; html += '' + ts_str + ''; - html += '' + (msg.level || "") + ''; - if (msg.host) html += '' + msg.host + ''; - if (msg.service) html += '' + msg.service + ''; - html += '' + msg.message + ''; + html += '' + escHtml(msg.level || "") + ''; + if (msg.host) html += '' + escHtml(msg.host) + ''; + if (msg.service) html += '' + escHtml(msg.service) + ''; + html += '' + escHtml(msg.message) + ''; html += '
'; msgs.insertAdjacentHTML(state.history ? "beforeend" : "afterbegin", html); applyLogFilters(); diff --git a/hbd/server/udp.py b/hbd/server/udp.py index e664185..5fb0037 100644 --- a/hbd/server/udp.py +++ b/hbd/server/udp.py @@ -424,7 +424,7 @@ def handle_datagram(msg: dict, addr, transport, ctx: dict): if plugin_name == "os_info": config_owner = config_mod.get_host_access(cfg, uname).get("owner") default_owner = config_mod.get_default_owner(cfg) - inferred_owner = plugin_data.get("owner", config_owner or default_owner) + inferred_owner = config_owner or plugin_data.get("owner") or default_owner host.owner = inferred_owner logger.info(f"owner for {uname} is {host.owner}") if DEBUG > 1: