[MEDIUM] Disk analyzer (k4dirstat/baobab) runs the df-derived mount point through a shell on 'Analyse Diskspace' #50

Summary

Linux.openDiskSpaceAnalyzer() launches k4dirstat/baobab with the disk's
mount point as its only argument, but does not pass runInShell: false —
so the call goes through Linux.runCommandWithCustomArguments's default
runInShell: true and Process.run joins the argv with spaces into a
sh -c command line. A mount point containing a space is split into two
arguments (the analyzer opens the wrong path); a mount point containing shell
metacharacters (;, $(), backticks, &, |) is executed as shell code by
/bin/sh the moment the user clicks Analyse Diskspace for that disk.

The mount point is not a constant: it comes from df -h output parsed in
lib/linux/linux_filesystem.dart, and for removable media it is derived from
the volume label — external data chosen by whoever formatted the stick.

• lib/services/linux.dart:2752 — runCommandWithCustomArguments("k4dirstat", [path])
• lib/services/linux.dart:2755 — same, after the install-queue branch
• lib/services/linux.dart:2770 — runCommandWithCustomArguments("baobab", [path])
• lib/linux/linux_filesystem.dart:68 — mount point = match.group(6) of the df -h line
• lib/layouts/disk_cleaner/clean_disk.dart:60 — Linux.openDiskSpaceAnalyzer(context, mountpoint)

Related: #42 covers the runInShell: true default at the execution layer and
enumerates other sinks (openfile:, openfolder:, openapp:, websearch, icon
loader); this sink was not in that list and is independently fixable.

Impact

Clicking "Analyse Diskspace" on a disk whose mount point carries shell
metacharacters executes attacker-influenced shell code as the desktop user.
The data source is third-party without being attacker-typed: for removable
volumes the mount directory name comes from the filesystem label
(/media/<user>/<label>/), which survives ; and $ sequences for several
filesystem types (udisks only excludes /). At minimum, every mount point
containing a space (/media/user/USB Stick/ — the exact case
parseDfOutput's comment says it was written to preserve) silently breaks:
the shell splits it into two arguments and the analyzer opens the wrong
directory. The queued fallback (LinuxCommand(argv: ["k4dirstat", "$path"]))
is argv-based and safe — only the three direct launches are affected.

Complexity

• Complexity: 2 (Easy)

Evidence

// lib/services/linux.dart:2750-2756
static void openDiskSpaceAnalyzer(context, path) {
  if (currentenvironment.desktop == DESKTOPS.KDE) {
    runCommandWithCustomArguments("k4dirstat", [path]);      // no runInShell: false
    ensureApplicationInstallation(["k4dirstat"]);
    if (commandQueue.isEmpty) {
      runCommandWithCustomArguments("k4dirstat", [path]);    // no runInShell: false

// lib/linux/linux_filesystem.dart:56-69
final match = linePattern.firstMatch(line);
...
devices.add(DeviceInfo(source, size, ..., match.group(6)!, ...));  // mount point

Data-flow trace: df -h → LinuxFilesystem.parseDfOutput (group(6) =
everything after the fifth field, deliberately kept whole so
"/media/user/USB Stick" survives parsing) → CleanDiskPage.mountpoint
(lib/layouts/disk_cleaner/clean_disk.dart:60) → openDiskSpaceAnalyzer →
Process.run("/usr/bin/k4dirstat", ["/media/user/USB Stick/"], runInShell: true)
→ /bin/sh -c "k4dirstat /media/user/USB Stick/".

Reproduce the shell join without the app:

dart run - <<'DART'
import 'dart:io';
void main() {
  // Stands in for Process.run("k4dirstat", [mountpoint], runInShell: true)
  Process.run('echo', ['k4dirstat', '/media/user/x;touch /tmp/pwned/'],
      runInShell: true);
}
DART

Recommended Fix

In lib/services/linux.dart, pass runInShell: false to the three direct
launches in openDiskSpaceAnalyzer (:2752, :2755, :2770). This is safe
by the codebase's own documented precondition: expandCommand yields an
absolute /usr/bin/... path and the argument is a single argv element that
needs no shell handling. The same one-line change pattern is already applied
elsewhere (runPythonScript, runPrivilegedPythonScript,
linux.dart:59/62/1478/1501). While there, keep the queued LinuxCommand
branches as they are — they are already argv-based.

Optionally (follow-up, not required for this issue): consider a broader
grep-audit of remaining runCommandWithCustomArguments/runCommand call
sites without runInShell: false, tracked under #42.

References

• Dart Process.run with runInShell: true: arguments are joined with
  spaces and executed via a shell —
  https://api.dart.dev/stable/dart-io/Process/run.html
• CWE-78: Improper Neutralization of Special Elements used in an OS Command
  ('OS Command Injection') — https://cwe.mitre.org/data/definitions/78.html
• OWASP Command Injection — https://owasp.org/www-community/attacks/Command_Injection

Validation

• attacker_source: mount point taken from df -h output; for removable media
  the mount directory name derives from the volume label set by whoever
  formatted the device (plus any fstab/network mount point)
• missing_guard: no runInShell: false on the direct k4dirstat/baobab
  launches; the path argument reaches Process.run(..., runInShell: true),
  which joins the argv into a sh -c command line unquoted
• sink_effect: shell code embedded in the mount point executes as the desktop
  user when the user clicks "Analyse Diskspace"; space-containing mount
  points split into two arguments and open the wrong directory
• preconditions: a mounted filesystem whose mount point contains shell
  metacharacters (removable volume label, or a hand-written fstab entry) or a
  space; the user opens Disk Cleaner and clicks "Analyse Diskspace" on that
  disk's page
• proof_anchors: lib/services/linux.dart:2752, lib/services/linux.dart:2755,
  lib/services/linux.dart:2770 (runCommandWithCustomArguments("k4dirstat", [path]) / ("baobab", [path]) without runInShell), data source
  lib/linux/linux_filesystem.dart:68, caller
  lib/layouts/disk_cleaner/clean_disk.dart:60
• suggested_validation: grep -n 'k4dirstat\|baobab' lib/services/linux.dart | grep -n runCommandWithCustomArguments — then, after the fix, the same call sites must show runInShell: false; a behavioural check: bash -c 'set -- k4dirstat "/media/u/x;touch /tmp/pwned"; sh -c "k4dirstat $*"' demonstrates the unquoted shell join executing the metacharacter payload