From 0e12b406de4c8bcc4f682d98f46957933872dd06 Mon Sep 17 00:00:00 2001 From: acidburns Date: Mon, 2 Feb 2026 21:07:37 +0100 Subject: [PATCH] Harden web UI auth, input handling, and SD path validation - Add optional Basic Auth with NVS-backed credentials and STA/AP flags; protect status, wifi, history, and download routes - Stop pre-filling WiFi/MQTT/Web UI password fields; keep stored secrets on blank and add clear-password checkboxes - Add HTML escaping + URL encoding helpers and apply to user-controlled strings; add unit test - Harden /sd/download path validation (prefix, length, dotdot, slashes) and log rejections - Enforce protocol version in LoRa receive and release GPIO14 before SD init - Update README security, SD, and GPIO sharing notes --- README.md | 23 ++- include/config.h | 4 + include/html_util.h | 6 + include/wifi_manager.h | 2 + src/html_util.cpp | 49 ++++++ src/lora_transport.cpp | 3 + src/main.cpp | 1 + src/web_server.cpp | 170 ++++++++++++++++++--- src/wifi_manager.cpp | 11 +- test/test_html_escape/test_html_escape.cpp | 21 +++ 10 files changed, 260 insertions(+), 30 deletions(-) create mode 100644 include/html_util.h create mode 100644 src/html_util.cpp create mode 100644 test/test_html_escape/test_html_escape.cpp diff --git a/README.md b/README.md index 66f099d..409ccff 100644 --- a/README.md +++ b/README.md @@ -31,10 +31,10 @@ Variants: - SCL: GPIO22 - I2C address: 0x68 - Battery ADC: GPIO35 (via on-board divider) -- **Role select**: GPIO14 (INPUT_PULLDOWN, sampled at boot) +- **Role select**: GPIO14 (INPUT_PULLDOWN, sampled at boot, **shared with SD SCK**) - HIGH = Sender - LOW/floating = Receiver -- **OLED control**: GPIO13 (INPUT_PULLDOWN, sender only) +- **OLED control**: GPIO13 (INPUT_PULLDOWN, sender only, **shared with SD CS**) - HIGH = force OLED on - LOW = allow auto-off after timeout - Not used on receiver (OLED always on) @@ -43,6 +43,9 @@ Variants: ### Notes on GPIOs - GPIO34/35/36/39 are input-only and have **no internal pullups/pulldowns**. - Strap pins (GPIO0/2/4/5/12/15) can affect boot; avoid for role or control jumpers. +- GPIO14 is shared between role select and SD SCK. **Do not attach the role jumper in Receiver mode if the SD card is connected/used**, and never force GPIO14 high when using SD. +- GPIO13 is shared between OLED control and SD CS. Avoid driving OLED control when SD is active. +- Receiver firmware releases GPIO14 to `INPUT` (no pulldown) after boot before SD SPI init. ## Firmware Roles ### Sender (battery-powered) @@ -262,10 +265,20 @@ inline constexpr uint16_t EXPECTED_SENDER_IDS[NUM_SENDERS] = { 0xF19C }; - `/wifi`: WiFi/MQTT/NTP config (AP and STA) - `/sender/`: per-sender details - Sender IDs on `/` are clickable (open sender page in a new tab). -- In STA mode, the UI is also available via the board’s IP/hostname on your WiFi network. +- In STA mode, the UI is also available via the board's IP/hostname on your WiFi network. - Main page shows SD card file listing (downloadable). - Sender page includes a history chart (power) with configurable range/resolution/mode. +## Security +- Basic Auth is supported for the web UI. In STA mode it is enabled by default; AP mode is optional. +- Config flags in `include/config.h`: + - `WEB_AUTH_REQUIRE_STA` (default `true`) + - `WEB_AUTH_REQUIRE_AP` (default `false`) + - `WEB_AUTH_DEFAULT_USER` / `WEB_AUTH_DEFAULT_PASS` +- Web credentials are stored in NVS. `/wifi`, `/sd/download`, `/history/*`, `/`, `/sender/*`, and `/manual` require auth when enabled. +- Password inputs are not prefilled. Leaving a password blank keeps the stored value; use the "clear password" checkbox to erase it. +- User-controlled strings are HTML-escaped before embedding in pages. + ## MQTT - Topic: `smartmeter//state` - QoS 0 @@ -303,6 +316,7 @@ Key timing settings in `include/config.h`: - `ENABLE_SD_LOGGING` / `PIN_SD_CS` - `SD_HISTORY_MAX_DAYS` / `SD_HISTORY_MIN_RES_MIN` - `SD_HISTORY_MAX_BINS` / `SD_HISTORY_TIME_BUDGET_MS` + - `WEB_AUTH_REQUIRE_STA` / `WEB_AUTH_REQUIRE_AP` / `WEB_AUTH_DEFAULT_USER` / `WEB_AUTH_DEFAULT_PASS` ## Limits & Known Constraints - **Compression**: MeterData uses lightweight RLE (good for JSON but not optimal). @@ -320,6 +334,7 @@ Optional CSV logging to microSD (FAT32) when `ENABLE_SD_LOGGING = true`. `ts_utc,p_w,p1_w,p2_w,p3_w,e_kwh,bat_v,bat_pct,rssi,snr,err_m,err_d,err_tx,err_last` - `err_last` is written as text (`meter`, `decode`, `loratx`) only on the last sample of a batch that reports an error. - Files are downloadable from the main UI page. +- Downloads only allow absolute paths under `/dd3/`, reject `..`, backslashes, and repeated slashes, and enforce a max path length. - History chart on sender page stream-parses CSVs and bins data in the background. - SD uses the on-board microSD SPI pins (CS=13, MOSI=15, SCK=14, MISO=2). @@ -341,7 +356,7 @@ Optional CSV logging to microSD (FAT32) when `ENABLE_SD_LOGGING = true`. - `src/main.cpp`: role detection and main loop ## Quick Start -1. Set role jumper on GPIO13: +1. Set role jumper on GPIO14: - LOW: sender - HIGH: receiver 2. OLED control on GPIO13: diff --git a/include/config.h b/include/config.h index 5bcc3c7..6c50496 100644 --- a/include/config.h +++ b/include/config.h @@ -87,6 +87,10 @@ constexpr uint16_t SD_HISTORY_MAX_BINS = 4000; constexpr uint16_t SD_HISTORY_TIME_BUDGET_MS = 10; constexpr const char *AP_SSID_PREFIX = "DD3-Bridge-"; constexpr const char *AP_PASSWORD = "changeme123"; +constexpr bool WEB_AUTH_REQUIRE_STA = true; +constexpr bool WEB_AUTH_REQUIRE_AP = false; +constexpr const char *WEB_AUTH_DEFAULT_USER = "admin"; +constexpr const char *WEB_AUTH_DEFAULT_PASS = "admin"; constexpr uint8_t NUM_SENDERS = 1; inline constexpr uint16_t EXPECTED_SENDER_IDS[NUM_SENDERS] = { diff --git a/include/html_util.h b/include/html_util.h new file mode 100644 index 0000000..5ed1db4 --- /dev/null +++ b/include/html_util.h @@ -0,0 +1,6 @@ +#pragma once + +#include + +String html_escape(const String &input); +String url_encode_component(const String &input); diff --git a/include/wifi_manager.h b/include/wifi_manager.h index e9012ca..74f288c 100644 --- a/include/wifi_manager.h +++ b/include/wifi_manager.h @@ -12,6 +12,8 @@ struct WifiMqttConfig { String mqtt_pass; String ntp_server_1; String ntp_server_2; + String web_user; + String web_pass; bool valid; }; diff --git a/src/html_util.cpp b/src/html_util.cpp new file mode 100644 index 0000000..dff470f --- /dev/null +++ b/src/html_util.cpp @@ -0,0 +1,49 @@ +#include "html_util.h" + +String html_escape(const String &input) { + String out; + out.reserve(input.length() + 8); + for (size_t i = 0; i < input.length(); ++i) { + char c = input[i]; + switch (c) { + case '&': + out += "&"; + break; + case '<': + out += "<"; + break; + case '>': + out += ">"; + break; + case '"': + out += """; + break; + case '\'': + out += "'"; + break; + default: + out += c; + break; + } + } + return out; +} + +String url_encode_component(const String &input) { + String out; + out.reserve(input.length() * 3); + const char *hex = "0123456789ABCDEF"; + for (size_t i = 0; i < input.length(); ++i) { + unsigned char c = static_cast(input[i]); + bool safe = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || c == '-' || c == '_' || c == '.' || c == '~'; + if (safe) { + out += static_cast(c); + } else { + out += '%'; + out += hex[(c >> 4) & 0x0F]; + out += hex[c & 0x0F]; + } + } + return out; +} diff --git a/src/lora_transport.cpp b/src/lora_transport.cpp index 7ea0612..97032f6 100644 --- a/src/lora_transport.cpp +++ b/src/lora_transport.cpp @@ -85,6 +85,9 @@ bool lora_receive(LoraPacket &pkt, uint32_t timeout_ms) { if (crc_calc != crc_rx) { return false; } + if (buffer[0] != PROTOCOL_VERSION) { + return false; + } pkt.protocol_version = buffer[0]; pkt.role = static_cast(buffer[1]); diff --git a/src/main.cpp b/src/main.cpp index 32e3fc8..e173807 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -626,6 +626,7 @@ void setup() { update_battery_cache(); } else { power_receiver_init(); + pinMode(PIN_ROLE, INPUT); // release pulldown before SD uses GPIO14 as SCK sd_logger_init(); wifi_manager_init(); init_sender_statuses(); diff --git a/src/web_server.cpp b/src/web_server.cpp index 7fdc81b..c5d2bc7 100644 --- a/src/web_server.cpp +++ b/src/web_server.cpp @@ -4,6 +4,7 @@ #include "config.h" #include "sd_logger.h" #include "time_manager.h" +#include "html_util.h" #include #include #include @@ -15,6 +16,8 @@ static const SenderStatus *g_statuses = nullptr; static uint8_t g_status_count = 0; static WifiMqttConfig g_config; static bool g_is_ap = false; +static String g_web_user; +static String g_web_pass; static const FaultCounters *g_sender_faults = nullptr; static const FaultType *g_sender_last_errors = nullptr; static MeterData g_last_batch[NUM_SENDERS][METER_BATCH_MAX_SAMPLES]; @@ -50,11 +53,30 @@ struct HistoryJob { static HistoryJob g_history = {}; static constexpr size_t SD_LIST_MAX_FILES = 200; +static constexpr size_t SD_DOWNLOAD_MAX_PATH = 160; + +static bool auth_required() { + return g_is_ap ? WEB_AUTH_REQUIRE_AP : WEB_AUTH_REQUIRE_STA; +} + +static bool ensure_auth() { + if (!auth_required()) { + return true; + } + const char *user = g_web_user.c_str(); + const char *pass = g_web_pass.c_str(); + if (server.authenticate(user, pass)) { + return true; + } + server.requestAuthentication(BASIC_AUTH, "DD3", "Authentication required"); + return false; +} static String html_header(const String &title) { + String safe_title = html_escape(title); String h = ""; - h += "" + title + ""; - h += "

" + title + "

"; + h += "" + safe_title + ""; + h += "

" + safe_title + "

"; return h; } @@ -78,6 +100,46 @@ static String format_faults(uint8_t idx) { return s; } +static bool sanitize_sd_download_path(String &path, String &error) { + path.trim(); + if (path.length() == 0) { + error = "empty"; + return false; + } + if (path.startsWith("dd3/")) { + path = "/" + path; + } + if (path.length() > SD_DOWNLOAD_MAX_PATH) { + error = "too_long"; + return false; + } + if (!path.startsWith("/dd3/")) { + error = "prefix"; + return false; + } + if (path.indexOf("..") >= 0) { + error = "dotdot"; + return false; + } + if (path.indexOf('\\') >= 0) { + error = "backslash"; + return false; + } + if (path.indexOf("//") >= 0) { + error = "repeated_slash"; + return false; + } + return true; +} + +static bool checkbox_checked(const char *name) { + if (!server.hasArg(name)) { + return false; + } + String val = server.arg(name); + return val == "on" || val == "true" || val == "1"; +} + static void history_reset() { if (g_history.file) { g_history.file.close(); @@ -228,7 +290,9 @@ static String render_sender_block(const SenderStatus &status) { } } String device_id = status.last_data.device_id; - s += "" + device_id + ""; + String device_id_safe = html_escape(device_id); + String device_id_url = url_encode_component(device_id); + s += "" + device_id_safe + ""; if (status.has_data && status.last_data.link_valid) { s += " RSSI:" + String(status.last_data.link_rssi_dbm) + " SNR:" + String(status.last_data.link_snr_db, 1); } @@ -271,14 +335,15 @@ static void append_sd_listing(String &html, const String &dir_path, uint8_t dept } } if (entry.isDirectory()) { - html += "
  • " + full_path + "/
    • "; + html += "
  • " + html_escape(full_path) + "/
    • "; append_sd_listing(html, full_path, depth + 1, count); } else { String href = full_path; if (!href.startsWith("/")) { href = "/" + href; } - html += "
  • " + full_path + ""; + String href_enc = url_encode_component(href); + html += "
  • " + html_escape(full_path) + ""; html += " (" + String(entry.size()) + " bytes)
    • "; count++; } @@ -288,6 +353,9 @@ static void append_sd_listing(String &html, const String &dir_path, uint8_t dept } static void handle_root() { + if (!ensure_auth()) { + return; + } String html = html_header("DD3 Bridge Status"); html += g_is_ap ? "

    Mode: AP

    " : "

    Mode: STA

    "; @@ -316,16 +384,26 @@ static void handle_root() { } static void handle_wifi_get() { + if (!ensure_auth()) { + return; + } String html = html_header("WiFi/MQTT Config"); html += "
    "; - html += "SSID:
    "; - html += "Password:
    "; - html += "MQTT Host:
    "; + html += "SSID:
    "; + html += "Password: "; + html += "
    "; + html += "MQTT Host:
    "; html += "MQTT Port:
    "; - html += "MQTT User:
    "; - html += "MQTT Pass:
    "; - html += "NTP Server 1:
    "; - html += "NTP Server 2:
    "; + html += "MQTT User:
    "; + html += "MQTT Pass: "; + html += "
    "; + html += "NTP Server 1:
    "; + html += "NTP Server 2:
    "; + html += "
    "; + html += "Web UI User:
    "; + html += "Web UI Pass: "; + html += "
    "; + html += "
    Leaving password blank keeps the existing one.
    "; html += ""; html += "
    "; html += html_footer(); @@ -333,15 +411,38 @@ static void handle_wifi_get() { } static void handle_wifi_post() { - WifiMqttConfig cfg; - cfg.ntp_server_1 = "pool.ntp.org"; - cfg.ntp_server_2 = "time.nist.gov"; + if (!ensure_auth()) { + return; + } + WifiMqttConfig cfg = g_config; + cfg.ntp_server_1 = g_config.ntp_server_1.length() > 0 ? g_config.ntp_server_1 : "pool.ntp.org"; + cfg.ntp_server_2 = g_config.ntp_server_2.length() > 0 ? g_config.ntp_server_2 : "time.nist.gov"; cfg.ssid = server.arg("ssid"); - cfg.password = server.arg("pass"); + String wifi_pass = server.arg("pass"); + if (checkbox_checked("clear_wifi_pass")) { + cfg.password = ""; + } else if (wifi_pass.length() > 0) { + cfg.password = wifi_pass; + } cfg.mqtt_host = server.arg("mqhost"); cfg.mqtt_port = static_cast(server.arg("mqport").toInt()); cfg.mqtt_user = server.arg("mquser"); - cfg.mqtt_pass = server.arg("mqpass"); + String mqtt_pass = server.arg("mqpass"); + if (checkbox_checked("clear_mqtt_pass")) { + cfg.mqtt_pass = ""; + } else if (mqtt_pass.length() > 0) { + cfg.mqtt_pass = mqtt_pass; + } + String web_user = server.arg("webuser"); + if (web_user.length() > 0) { + cfg.web_user = web_user; + } + String web_pass = server.arg("webpass"); + if (checkbox_checked("clear_web_pass")) { + cfg.web_pass = ""; + } else if (web_pass.length() > 0) { + cfg.web_pass = web_pass; + } if (server.arg("ntp1").length() > 0) { cfg.ntp_server_1 = server.arg("ntp1"); } @@ -349,6 +450,9 @@ static void handle_wifi_post() { cfg.ntp_server_2 = server.arg("ntp2"); } cfg.valid = true; + g_config = cfg; + g_web_user = cfg.web_user; + g_web_pass = cfg.web_pass; wifi_save_config(cfg); server.send(200, "text/html", "Saved. Rebooting..."); delay(1000); @@ -356,12 +460,16 @@ static void handle_wifi_post() { } static void handle_sender() { + if (!ensure_auth()) { + return; + } if (!g_statuses) { server.send(404, "text/plain", "No senders"); return; } String uri = server.uri(); String device_id = uri.substring(String("/sender/").length()); + String device_id_url = url_encode_component(device_id); for (uint8_t i = 0; i < g_status_count; ++i) { if (device_id.equalsIgnoreCase(g_statuses[i].last_data.device_id)) { String html = html_header("Sender " + device_id); @@ -376,6 +484,7 @@ static void handle_sender() { html += ""; html += ""; html += "