Skip to content

Commit 5703fd2

Browse files
fhirschmannclaude
andcommitted
fix(web): close unauth upload/OTA, password leak and SSID XSS
The password protection was implemented as server middleware, which only wraps the request handler - ESPAsyncWebServer runs the upload/body callback while the body streams in, *before* the middleware. So /update (OTA flash), /upload (NVS restore) and POST /explorer (SD write) were reachable unauthenticated, i.e. an unauthenticated firmware flash = remote code execution even with a web password set. - add Web_RequestAuthorized() and enforce it inside the OTA, NVS-restore and explorer upload handlers (re-checked on every chunk) plus their completion handlers, so nothing touches flash/NVS/SD before auth. - GET /security no longer returns the device password in cleartext (it is the shared web/FTP/WebDAV credential); the UI field is now write-only and gets an explicit "disable protection" switch so an empty save can't silently turn protection off. Locales + demo mock updated. - escape scanned SSIDs: build the AP-setup WiFi list via DOM/textContent instead of innerHTML, and JSON-escape SSID/BSSID server-side, closing the DOM-XSS on the page that takes the WiFi password. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 00651bf commit 5703fd2

9 files changed

Lines changed: 158 additions & 27 deletions

File tree

‎html/accesspoint.html‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,14 +283,28 @@ <h1 data-i18n="wifi.restartPrompt"></h1>
283283
}
284284

285285
function listWiFi(data) {
286-
console.log(data);
287286
var list = document.getElementById("WiFiList");
288-
var h = "";
289-
data.map(function (WiFis) {
290-
h += `<div class="ssid" onclick="selectWiFi('${WiFis.ssid}')" title="${WiFis.quality}% (${WiFis.rssi} dBm) ${WiFis.bssid}"><span class="ico ${WiFis.wico}"></span><span class="name">${WiFis.ssid}</span><span class="ico ${WiFis.pico}"></span></div>\n`;
287+
// Build the list via DOM nodes + textContent, never innerHTML: the SSID/BSSID are fully
288+
// attacker-controlled (any nearby AP can broadcast a crafted 32-byte SSID), so string
289+
// interpolation here would be DOM-based XSS on the page that takes the WiFi password.
290+
list.textContent = "";
291+
data.forEach(function (WiFis) {
292+
var row = document.createElement("div");
293+
row.className = "ssid";
294+
row.title = WiFis.quality + "% (" + WiFis.rssi + " dBm) " + (WiFis.bssid || "");
295+
row.addEventListener("click", function () { selectWiFi(WiFis.ssid); });
296+
var wico = document.createElement("span");
297+
wico.className = "ico " + WiFis.wico;
298+
var name = document.createElement("span");
299+
name.className = "name";
300+
name.textContent = WiFis.ssid;
301+
var pico = document.createElement("span");
302+
pico.className = "ico " + WiFis.pico;
303+
row.appendChild(wico);
304+
row.appendChild(name);
305+
row.appendChild(pico);
306+
list.appendChild(row);
291307
});
292-
h += "<br>";
293-
list.innerHTML = h;
294308
setTimeout(getWiFiList, 5000);
295309
}
296310
// disable static input fields on toggle

‎html/locales/de.json‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,12 @@
210210
"title": "Geräte-Passwort",
211211
"password": {
212212
"title": "Passwort",
213-
"placeholder": "Leer lassen, um den Schutz zu deaktivieren",
214-
"desc": "Ein gemeinsames Passwort für alles: Webinterface, FTP und WebDAV. Bei FTP und WebDAV ist der Benutzername egal – jeder Benutzername funktioniert, es zählt nur dieses Passwort. Nach dem Anmelden im Webinterface bleibt man 90 Tage eingeloggt; im Hotspot-Modus ist der Schutz inaktiv. Leer lassen, um den Schutz überall (Webinterface, FTP, WebDAV) zu deaktivieren.",
215-
"show": "Passwort anzeigen/verbergen"
213+
"placeholder": "Passwort festlegen",
214+
"keep": "Leer lassen, um das aktuelle Passwort zu behalten",
215+
"desc": "Ein gemeinsames Passwort für alles: Webinterface, FTP und WebDAV. Bei FTP und WebDAV ist der Benutzername egal – jeder Benutzername funktioniert, es zählt nur dieses Passwort. Nach dem Anmelden im Webinterface bleibt man 90 Tage eingeloggt; im Hotspot-Modus ist der Schutz inaktiv. Das Passwort wird nie wieder angezeigt; leer lassen, um es zu behalten. Zum Deaktivieren überall „Passwortschutz deaktivieren“ anhaken und speichern.",
216+
"show": "Passwort anzeigen/verbergen",
217+
"disable": "Passwortschutz deaktivieren",
218+
"nochange": "Nichts zu speichern: neues Passwort eingeben oder „Passwortschutz deaktivieren“ anhaken."
216219
},
217220
"cookie": {
218221
"title": "Anmeldung merken",

‎html/locales/en.json‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,12 @@
210210
"title": "Device password",
211211
"password": {
212212
"title": "Password",
213-
"placeholder": "Leave empty to disable protection",
214-
"desc": "One shared password for everything: web interface, FTP and WebDAV. For FTP and WebDAV the username does not matter - any username works, only this password is checked. Once logged in to the web interface you stay logged in for 90 days; protection is inactive in hotspot mode. Leave empty to disable protection everywhere (web interface, FTP, WebDAV).",
215-
"show": "Show/hide password"
213+
"placeholder": "Set a password",
214+
"keep": "Leave empty to keep the current password",
215+
"desc": "One shared password for everything: web interface, FTP and WebDAV. For FTP and WebDAV the username does not matter - any username works, only this password is checked. Once logged in to the web interface you stay logged in for 90 days; protection is inactive in hotspot mode. The password is never shown again; leave the field empty to keep it. To turn protection off everywhere, tick “Disable password protection” and save.",
216+
"show": "Show/hide password",
217+
"disable": "Disable password protection",
218+
"nochange": "Nothing to save: enter a new password or tick “Disable password protection”."
216219
},
217220
"cookie": {
218221
"title": "Stay logged in",

‎html/locales/fr.json‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,12 @@
203203
"title": "Mot de passe de l'appareil",
204204
"password": {
205205
"title": "Mot de passe",
206-
"placeholder": "Laisser vide pour désactiver la protection",
207-
"desc": "Un seul mot de passe partagé pour tout : interface web, FTP et WebDAV. Pour le FTP et le WebDAV, le nom d'utilisateur n'a pas d'importance - n'importe quel nom fonctionne, seul ce mot de passe est vérifié. Une fois connecté à l'interface web, vous restez connecté pendant 90 jours ; la protection est inactive en mode hotspot. Laisser vide pour désactiver la protection partout (interface web, FTP, WebDAV).",
208-
"show": "Afficher/masquer le mot de passe"
206+
"placeholder": "Définir un mot de passe",
207+
"keep": "Laisser vide pour conserver le mot de passe actuel",
208+
"desc": "Un seul mot de passe partagé pour tout : interface web, FTP et WebDAV. Pour le FTP et le WebDAV, le nom d'utilisateur n'a pas d'importance - n'importe quel nom fonctionne, seul ce mot de passe est vérifié. Une fois connecté à l'interface web, vous restez connecté pendant 90 jours ; la protection est inactive en mode hotspot. Le mot de passe n'est jamais réaffiché ; laissez le champ vide pour le conserver. Pour tout désactiver, cochez « Désactiver la protection par mot de passe » puis enregistrez.",
209+
"show": "Afficher/masquer le mot de passe",
210+
"disable": "Désactiver la protection par mot de passe",
211+
"nochange": "Rien à enregistrer : saisissez un nouveau mot de passe ou cochez « Désactiver la protection par mot de passe »."
209212
},
210213
"cookie": {
211214
"title": "Rester connecté",

‎html/management.html‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3421,6 +3421,10 @@ <h5 class="modal-title" data-i18n="tools.nvs.erase.title"></h5>
34213421
</div>
34223422
<small class="form-text" data-i18n="security.password.desc"></small>
34233423
</div>
3424+
<div class="form-check form-switch mb-3">
3425+
<input class="form-check-input" type="checkbox" id="securityDisable" name="securityDisable">
3426+
<label class="form-check-label" for="securityDisable" data-i18n="security.password.disable"></label>
3427+
</div>
34243428
<div class="text-center">
34253429
<button type="submit" class="btn btn-primary" data-i18n="submit"></button>
34263430
</div>
@@ -7506,9 +7510,12 @@ <h5 class="modal-title" data-i18n="tools.nvs.erase.title"></h5>
75067510
try {
75077511
const status = await apiGet("/security");
75087512
document.getElementById('menuLogout').setAttribute('data-visible', status.enabled === true);
7509-
if (status.password !== undefined) {
7510-
document.getElementById('wwwPassword').value = status.password;
7511-
}
7513+
// the password is write-only: the device no longer echoes it back, so the field stays
7514+
// empty (empty = keep current). Disabling protection is an explicit opt-in switch.
7515+
document.getElementById('wwwPassword').value = "";
7516+
document.getElementById('securityDisable').checked = false;
7517+
const pwPlaceholder = status.enabled === true ? 'security.password.keep' : 'security.password.placeholder';
7518+
document.getElementById('wwwPassword').setAttribute('placeholder', i18next.t(pwPlaceholder));
75127519
if (status.cookieDays !== undefined) {
75137520
const unlimited = (status.cookieDays === 0);
75147521
document.getElementById('cookieUnlimited').checked = unlimited;
@@ -7544,14 +7551,25 @@ <h5 class="modal-title" data-i18n="tools.nvs.erase.title"></h5>
75447551
}
75457552

75467553
async function securityConfig() {
7554+
const pw = document.getElementById('wwwPassword').value;
7555+
const disable = document.getElementById('securityDisable').checked;
7556+
const payload = {};
7557+
if (disable) {
7558+
// explicit opt-in: send an empty password to turn protection off everywhere
7559+
payload.password = "";
7560+
} else if (pw.length > 0) {
7561+
payload.password = pw;
7562+
} else {
7563+
// empty field + disable not ticked = nothing to change (don't accidentally clear it)
7564+
toaster.error(i18next.t('security.password.nochange'));
7565+
return;
7566+
}
75477567
const response = await fetch("http://" + host + "/security", {
75487568
method: "POST",
75497569
headers: {
75507570
"Content-Type": "application/json"
75517571
},
7552-
body: JSON.stringify({
7553-
password: document.getElementById('wwwPassword').value
7554-
})
7572+
body: JSON.stringify(payload)
75557573
});
75567574
if (response.ok) {
75577575
flashActionOk();

‎src/Web.cpp‎

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,43 @@ bool Web_DumpNvsToSd(const char *_namespace, const char *_destFile) {
224224

225225
// First request will return 0 results unless you start scan from somewhere else (loop/setup)
226226
// Do not request more often than 3-5 seconds
227+
// Minimal JSON string escaper for the hand-concatenated JSON bodies in this file. SSIDs (and other
228+
// external strings) are attacker-controlled, so an unescaped quote/backslash/control char would
229+
// otherwise produce invalid JSON (client parse fails) or, in an innerHTML context, enable XSS.
230+
static String Web_JsonEscape(const String &in) {
231+
String out;
232+
out.reserve(in.length() + 8);
233+
for (size_t i = 0; i < in.length(); i++) {
234+
char c = in[i];
235+
switch (c) {
236+
case '"':
237+
out += "\\\"";
238+
break;
239+
case '\\':
240+
out += "\\\\";
241+
break;
242+
case '\n':
243+
out += "\\n";
244+
break;
245+
case '\r':
246+
out += "\\r";
247+
break;
248+
case '\t':
249+
out += "\\t";
250+
break;
251+
default:
252+
if ((uint8_t) c < 0x20) {
253+
char u[7];
254+
snprintf(u, sizeof(u), "\\u%04x", (uint8_t) c);
255+
out += u;
256+
} else {
257+
out += c;
258+
}
259+
}
260+
}
261+
return out;
262+
}
263+
227264
static void handleWiFiScanRequest(AsyncWebServerRequest *request) {
228265
String json = "[";
229266
int n = WiFi.scanComplete();
@@ -248,8 +285,8 @@ static void handleWiFiScanRequest(AsyncWebServerRequest *request) {
248285
json += ",";
249286
}
250287
json += "{";
251-
json += "\"ssid\":\"" + WiFi.SSID(i) + "\"";
252-
json += ",\"bssid\":\"" + WiFi.BSSIDstr(i) + "\"";
288+
json += "\"ssid\":\"" + Web_JsonEscape(WiFi.SSID(i)) + "\"";
289+
json += ",\"bssid\":\"" + Web_JsonEscape(WiFi.BSSIDstr(i)) + "\"";
253290
json += ",\"rssi\":" + String(WiFi.RSSI(i));
254291
json += ",\"channel\":" + String(WiFi.channel(i));
255292
json += ",\"secure\":" + String(WiFi.encryptionType(i));
@@ -395,6 +432,18 @@ static bool Web_IsAuthenticated(AsyncWebServerRequest *request) {
395432
return token.equals(wwwSessionToken);
396433
}
397434

435+
// Public auth gate for streaming body/upload handlers. The password middleware only wraps the
436+
// request *handler*; ESPAsyncWebServer invokes the upload/body callback while the body is still
437+
// streaming in, i.e. BEFORE the middleware chain runs. So /update (OTA), /upload (NVS restore)
438+
// and POST /explorer (SD write) must enforce auth themselves before touching flash/NVS/SD.
439+
// Mirrors the middleware: allowed in AP-mode or when no password is configured.
440+
bool Web_RequestAuthorized(AsyncWebServerRequest *request) {
441+
if (WiFi.getMode() == WIFI_AP) {
442+
return true;
443+
}
444+
return Web_IsAuthenticated(request);
445+
}
446+
398447
static void Web_HandlePostLogin(AsyncWebServerRequest *request, JsonVariant &json) {
399448
if (wwwLoginLockedUntil && (millis() < wwwLoginLockedUntil)) {
400449
request->send(429);
@@ -530,9 +579,9 @@ void webserverStart(void) {
530579
JsonDocument doc;
531580
doc["enabled"] = (wwwSessionToken.length() > 0);
532581
doc["cookieDays"] = gPrefsSettings.getUInt("wwwCookieDays", WWW_COOKIE_DAYS_DEFAULT);
533-
// The whole management page already sits behind this very password (same as the FTP tab,
534-
// which also returns its password), so the UI can show + reveal the current value.
535-
doc["password"] = gPrefsSettings.getString("wwwPassword", "");
582+
// The password is deliberately NOT returned: it is the shared web/FTP/WebDAV credential
583+
// and echoing it back turns every authenticated read (or any XSS/CSRF that can fetch
584+
// /security) into a plaintext credential leak. The field in the UI is write-only.
536585
String out;
537586
serializeJson(doc, out);
538587
request->send(200, "application/json", out);
@@ -634,13 +683,24 @@ void webserverStart(void) {
634683
// NVS-backup-upload
635684
wServer.on(
636685
"/upload", HTTP_POST, [](AsyncWebServerRequest *request) {
686+
// the body handler (handleUpload) runs before the middleware, so re-check auth here
687+
if (!Web_RequestAuthorized(request)) {
688+
request->send(401);
689+
return;
690+
}
637691
request->send(200);
638692
},
639693
handleUpload);
640694

641695
// OTA-upload
642696
wServer.on(
643697
"/update", HTTP_POST, [](AsyncWebServerRequest *request) {
698+
// the upload handler streams (and flashes) the body before the middleware runs;
699+
// guard the completion the same way the body handler guards the actual flash
700+
if (!Web_RequestAuthorized(request)) {
701+
request->send(401);
702+
return;
703+
}
644704
#ifdef BOARD_HAS_16MB_FLASH_AND_OTA_SUPPORT
645705
if (Update.hasError()) {
646706
request->send(500, "text/plain", Update.errorString());
@@ -656,6 +716,12 @@ void webserverStart(void) {
656716
Log_Println(otaNotSupported, LOGLEVEL_NOTICE);
657717
return;
658718
#endif
719+
// CRITICAL: the middleware does NOT cover this streaming upload callback, so an
720+
// unauthenticated client could otherwise flash arbitrary firmware. Refuse to touch
721+
// Update unless the request is authorized (re-checked on every chunk).
722+
if (!Web_RequestAuthorized(request)) {
723+
return;
724+
}
659725

660726
if (!index) {
661727
// pause some tasks to get more free CPU time for the upload
@@ -973,6 +1039,11 @@ void webserverStart(void) {
9731039
"/explorer", HTTP_POST, [](AsyncWebServerRequest *request) {
9741040
// clean up buffers
9751041
request->onDisconnect([]() { destroyDoubleBuffer(); });
1042+
// the upload handler writes to SD before the middleware runs, so re-check auth here
1043+
if (!Web_RequestAuthorized(request)) {
1044+
request->send(401);
1045+
return;
1046+
}
9761047
// we are finished with the upload
9771048
if (!request->_tempObject) {
9781049
request->send(200);

‎src/WebExplorer.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,13 @@ void handleUploadError(AsyncWebServerRequest *request, int code) {
118118
// requires a GET parameter path, as directory path to the file
119119
void explorerHandleFileUpload(AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) {
120120

121+
// This upload callback runs while the body streams in, before the password middleware. Refuse
122+
// to write anything to SD for an unauthenticated client; the completion handler returns 401.
123+
if (!Web_RequestAuthorized(request)) {
124+
uploadAborted = true;
125+
return;
126+
}
127+
121128
System_UpdateActivityTimer();
122129

123130
// New File
@@ -698,6 +705,11 @@ void handleUpload(AsyncWebServerRequest *request, String filename, size_t index,
698705
static File tmpFile;
699706
static size_t fileIndex = 0;
700707
static char tmpFileName[13];
708+
// Runs before the password middleware (body streams in first), so an unauthenticated client
709+
// could otherwise inject RFID assignments into NVS. Refuse to buffer or import anything.
710+
if (!Web_RequestAuthorized(request)) {
711+
return;
712+
}
701713
esp_task_wdt_reset();
702714
if (!index) {
703715
snprintf(tmpFileName, 13, "/_%lu", millis());

‎src/WebInternal.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,8 @@ void Web_DumpSdToNvs(const char *_filename);
6262

6363
// Aborts a running file-upload storage task and waits for it to exit (used by Web_Exit).
6464
void WebExplorer_AbortUpload(void);
65+
66+
// Auth gate for streaming body/upload handlers (defined in Web.cpp). The password middleware
67+
// does not cover the upload callback, which runs while the body streams in, so handlers that
68+
// write flash/NVS/SD must call this themselves before doing anything.
69+
bool Web_RequestAuthorized(AsyncWebServerRequest *request);

‎tools/demo/demo-mock.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,8 @@
231231
if (p === "/activeSSID") { return jsonResp({ active: "Heimnetz" }); }
232232
if (p === "/eqrules") { return jsonResp([]); }
233233
if (p === "/homekit") { return jsonResp({ enabled: false, paired: false, code: "", deviceName: "ESPuino", tvName: "ESPuino TV" }); }
234+
// password status: the device never echoes the password back (write-only field)
235+
if (p === "/security") { return jsonResp({ enabled: false, cookieDays: 90 }); }
234236
if (p === "/bluetoothresults") { return jsonResp([]); }
235237
if (p === "/settings") {
236238
var section = qs.get("section");
@@ -280,7 +282,7 @@
280282
}
281283

282284
// Everything that writes / triggers an action on the device is a no-op in the demo.
283-
if (/^\/(restart|shutdown|githubupdate|settings|sync|syncstop|rfidsync|backupupload|rfidnvserase|rfidresetpos|explorer|exploreraudio|homekit|bluetoothscan|bluetoothconnect|upload|savedSSIDs|trackcontrol|volume|ftp|webdav|logout)\b/.test(p)) {
285+
if (/^\/(restart|shutdown|githubupdate|settings|sync|syncstop|rfidsync|backupupload|rfidnvserase|rfidresetpos|explorer|exploreraudio|homekit|security|bluetoothscan|bluetoothconnect|upload|savedSSIDs|trackcontrol|volume|ftp|webdav|logout)\b/.test(p)) {
284286
return jsonResp({ status: "ok", demo: true });
285287
}
286288
return null;

0 commit comments

Comments
 (0)