From 5b9285aeb9be26faf8c4621f8fea50507dc92e99 Mon Sep 17 00:00:00 2001 From: Alex Petrochenko Date: Tue, 19 May 2026 13:40:43 +0100 Subject: [PATCH] fix(web): validate inputs in handleConfigSave to prevent fuzz bricking (M1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously /config accepted any value and called ESP.restart() — fuzz tests (or any malicious POST) could save garbage SSIDs and brick the device until FTDI recovery. Now: - SSID: rejected if empty, >31 chars, non-printable, or all-same-char (HTTP 400) - Password: rejected if >63 chars - Hostname/city_name/ntp_server: length-validated - Numeric fields (timezone, brightness, intervals, lat/lon, display): clamped to safe ranges via constrain() Tested on hardware: ssid="AAAA..." now correctly returns HTTP 400 and preserves existing config. Device survives entire fuzz suite. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 11 ++++ firmware/weather_clock/config.h | 2 +- firmware/weather_clock/web_server.cpp | 73 +++++++++++++++++++++++---- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16d7c0d..c3cfa60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.9.7] - 2026-05-19 + +### Fixed + +- **Config endpoint accepted garbage values, bricking device** (M1): `/config` form handler + now validates all inputs. SSID rejected if empty, >31 chars, non-printable, or all-same-char + (fuzz garbage like "AAAA..."). Numeric fields are `constrain()`-ed to safe ranges + (`ntp_interval`/`weather_interval`: 60–86400, `brightness`: 0–7, `timezone`: ±12h, + `latitude`/`longitude`: physical ranges, `display_orientation`: 0–3). Invalid input + returns HTTP 400 instead of silently saving and rebooting. + ## [1.9.6] - 2026-05-18 ### Fixed diff --git a/firmware/weather_clock/config.h b/firmware/weather_clock/config.h index 3f6f6f8..121f5f8 100644 --- a/firmware/weather_clock/config.h +++ b/firmware/weather_clock/config.h @@ -9,7 +9,7 @@ #include // Firmware version -#define FIRMWARE_VERSION "1.9.6" +#define FIRMWARE_VERSION "1.9.7" // OLED I2C Configuration #define I2C_SDA 0 // GPIO0 (I2C Data) - SWAPPED! diff --git a/firmware/weather_clock/web_server.cpp b/firmware/weather_clock/web_server.cpp index add3c15..56d6a0f 100644 --- a/firmware/weather_clock/web_server.cpp +++ b/firmware/weather_clock/web_server.cpp @@ -283,39 +283,90 @@ void ICACHE_FLASH_ATTR handleConfig() { server.sendContent(""); } +// Validate SSID: 1-31 printable ASCII chars, not all-same-char (likely fuzz garbage) +static bool isValidSSID(const String& s) { + size_t n = s.length(); + if (n == 0 || n > 31) return false; + char first = s[0]; + bool allSame = true; + for (size_t i = 0; i < n; i++) { + char c = s[i]; + if (c < 0x20 || c > 0x7E) return false; // non-printable + if (c != first) allSame = false; + } + return !allSame; // reject "AAAAA...", "BBBBB...", etc. +} + void ICACHE_FLASH_ATTR handleConfigSave() { + // Validate before saving — reject obviously bad input rather than brick the device if (server.hasArg("ssid")) { - safeStringCopy(server.arg("ssid"), config.ssid, sizeof(config.ssid)); + String s = server.arg("ssid"); + if (!isValidSSID(s)) { + server.send(400, "text/plain", "Invalid SSID (1-31 printable chars, not all-same)"); + return; + } + safeStringCopy(s, config.ssid, sizeof(config.ssid)); } if (server.hasArg("password")) { - safeStringCopy(server.arg("password"), config.password, sizeof(config.password)); + String p = server.arg("password"); + if (p.length() > 63) { + server.send(400, "text/plain", "Password too long (max 63 chars)"); + return; + } + safeStringCopy(p, config.password, sizeof(config.password)); } if (server.hasArg("timezone")) { - config.timezone_offset = server.arg("timezone").toInt(); + long tz = server.arg("timezone").toInt(); + config.timezone_offset = constrain(tz, -43200L, 43200L); // ±12h } if (server.hasArg("brightness")) { - config.brightness = server.arg("brightness").toInt(); + config.brightness = constrain(server.arg("brightness").toInt(), 0, 7); } if (server.hasArg("hostname")) { - safeStringCopy(server.arg("hostname"), config.hostname, sizeof(config.hostname)); + String h = server.arg("hostname"); + if (h.length() == 0 || h.length() > 31) { + server.send(400, "text/plain", "Invalid hostname length (1-31)"); + return; + } + safeStringCopy(h, config.hostname, sizeof(config.hostname)); } if (server.hasArg("city_name")) { - safeStringCopy(server.arg("city_name"), config.city_name, sizeof(config.city_name)); + String c = server.arg("city_name"); + if (c.length() > 31) { + server.send(400, "text/plain", "City name too long (max 31)"); + return; + } + safeStringCopy(c, config.city_name, sizeof(config.city_name)); } if (server.hasArg("latitude")) { - config.latitude = server.arg("latitude").toFloat(); + float lat = server.arg("latitude").toFloat(); + config.latitude = constrain(lat, -90.0f, 90.0f); } if (server.hasArg("longitude")) { - config.longitude = server.arg("longitude").toFloat(); + float lon = server.arg("longitude").toFloat(); + config.longitude = constrain(lon, -180.0f, 180.0f); } if (server.hasArg("weather_interval")) { - config.weather_interval = server.arg("weather_interval").toInt(); + long wi = server.arg("weather_interval").toInt(); + config.weather_interval = constrain(wi, 60L, 86400L); // 1min to 1day + } + if (server.hasArg("ntp_interval")) { + long ni = server.arg("ntp_interval").toInt(); + config.ntp_interval = constrain(ni, 60L, 86400L); + } + if (server.hasArg("ntp_server")) { + String n = server.arg("ntp_server"); + if (n.length() == 0 || n.length() > 63) { + server.send(400, "text/plain", "Invalid NTP server (1-63 chars)"); + return; + } + safeStringCopy(n, config.ntp_server, sizeof(config.ntp_server)); } if (server.hasArg("display_rotation_sec")) { - config.display_rotation_sec = server.arg("display_rotation_sec").toInt(); + config.display_rotation_sec = constrain(server.arg("display_rotation_sec").toInt(), 1, 60); } if (server.hasArg("display_orientation")) { - config.display_orientation = server.arg("display_orientation").toInt(); + config.display_orientation = constrain(server.arg("display_orientation").toInt(), 0, 3); display.setRotation(config.display_orientation); }