[LOW] preferred_browser config value is used unvalidated as the browser executable (AppLauncher) #47

Summary

AppLauncher reads the config key preferred_browser and hands it, unvalidated, to the browser-detection chain, where it becomes the executable that is launched:

• lib/services/app_launcher.dart:120-125 — _configuredBrowser() returns ConfigHandler().getValueUnsafe('preferred_browser', '') with no validation at the boundary
• lib/services/app_launcher.dart:84-97 — detectBrowser() prepends the value ahead of the kKnownBrowsers allowlist without ever checking membership: final candidates = [ if (configured != null && configured.isNotEmpty) configured, ...kKnownBrowsers ];
• lib/services/app_launcher.dart:71 — _defaultStarter executes it: await Process.start(binary, args, mode: ProcessStartMode.detached);

So any string the config file contains becomes a binary name (resolved through which/PATH by _which, line 55-63) or an absolute path, and is launched as a detached process when the hub's browser button is used (lib/layouts/hub/hub_shell.dart:171-172, :420). The kKnownBrowsers allowlist exists but is only consulted after the unvalidated value — the config value bypasses its own allowlist.

This violates the repo's own invariant that every value read back from the config is validated before it reaches an interpreter (the same boundary the folder_recursion_depth fix enforces; the value is only user-writable today, so this is a boundary-validation gap, not a privilege crossing).

Impact

Any value in preferred_browser — set by a hand edit, a future settings UI, or any other process that can write ~/.config/linux-assistant/config.json — is launched as an arbitrary executable with the desktop user's privileges instead of a browser. It also silently breaks normal use: a stale or misspelled entry ("brave-browser " with a trailing space) is launched blindly and the failed path is only reached if the process cannot be spawned. Closing it makes the config key behave like the rest of the settings surface, where values are validated constants.

Complexity

• Complexity: 2 (Easy)

Evidence

// lib/services/app_launcher.dart:84-97
final configured = _configuredBrowser();
final candidates = [
  if (configured != null && configured.isNotEmpty) configured,
  ...kKnownBrowsers,
];
for (final bin in candidates) {
  if (await _which(bin)) return bin;
}

// lib/services/app_launcher.dart:69-73
static Future<bool> _defaultStarter(String binary, List<String> args) async {
  try {
    await Process.start(binary, args, mode: ProcessStartMode.detached);

// lib/services/app_launcher.dart:120-125
static String? _configuredBrowser() {
  try {
    final value = ConfigHandler().getValueUnsafe('preferred_browser', '');
    return value.isEmpty ? null : value;

No code in the repo ever writes preferred_browser (verified: the only reference to the key is the read above), so today the only source is the user's own config file. Launch flow: hub_shell.dart:420 → _launchBrowser() → launchBrowser() → detectBrowser() → _starter(configured, args).

Recommended Fix

Validate at the boundary in _configuredBrowser() — return the configured value only if it is a known browser:

static String? _configuredBrowser() {
  try {
    final value = ConfigHandler().getValueUnsafe('preferred_browser', '');
    if (value.isEmpty) return null;
    // Only a known binary name is honored; anything else falls back to the
    // kKnownBrowsers list and the xdg-open fallback.
    return kKnownBrowsers.contains(value.trim()) ? value.trim() : null;
  } catch (_) {
    return null;
  }
}

(Optionally also if (!value.contains('/')) to forbid absolute paths, and add a unit test in test/app_launcher_test.dart asserting that a preferred_browser of arbitrary-binary is ignored while firefox is honored.)

References

• CWE-78 argument-injection/unsafe-exec family: https://cwe.mitre.org/data/definitions/78.html
• OWASP Command Injection Defense Cheat Sheet (treat every external value as data, never as a program name): https://cheatsheetseries.owasp.org/cheatsheets/OS_Command_Injection_Defense_Cheat_Sheet.html
• Repo precedent: same config-boundary validation class as the folder_recursion_depth finding ([LOW] folder_recursion_depth config value is interpolated unvalidated into a bash -c find script #46).

Validation

attacker_source: the user-writable config file ~/.config/linux-assistant/config.json (key `preferred_browser`), or any other process able to write that file
missing_guard: the value is never checked against the kKnownBrowsers allowlist or restricted in any other way before becoming the executable name in Process.start
sink_effect: Process.start(binary, args, mode: detached) (lib/services/app_launcher.dart:71) launches an arbitrary PATH-resolvable binary or absolute path with the desktop user's privileges whenever the hub browser button is pressed
preconditions: `preferred_browser` must contain the crafted value in the config file, and the named binary must exist (absolute path) or resolve on PATH; today only the user themselves (or a process already running as them) can write the config, so no privilege boundary is crossed
proof_anchors: lib/services/app_launcher.dart:120-125 (read without validation), lib/services/app_launcher.dart:84-97 (value prepended ahead of the allowlist without membership check), lib/services/app_launcher.dart:69-73 (arbitrary binary executed)
suggested_validation: grep -n "kKnownBrowsers\|_configuredBrowser\|Process.start" lib/services/app_launcher.dart