From 76508cdea745c7ad9ccb039a8300788729c6a774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Mon, 12 Oct 2026 18:45:09 +0300 Subject: [PATCH 1/4] Prevent Open With filenames from expanding into shell code Co-Authored-By: Codex Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../Launching/DesktopExecExpander.cs | 158 +++++++++--------- .../Launching/DesktopExecExpanderTests.cs | 22 ++- .../Launching/DesktopExecInjectionTests.cs | 55 +++--- .../Launching/LauncherServiceTests.cs | 10 ++ 4 files changed, 129 insertions(+), 116 deletions(-) diff --git a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs index 5bdc356b6839..9ec0765d4e77 100644 --- a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs +++ b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs @@ -80,31 +80,92 @@ public static class DesktopExecExpander } /// - /// Expands the application's Exec into one or more argument vectors for the given targets (paths or URIs). - /// Field codes are substituted into the raw Exec string as shell-quoted words before tokenizing, so a value - /// stays one literal word, including inside sh -c "..." scripts. The first element of each vector is the program. - /// Returns an empty list when the Exec is empty or has unbalanced quotes. + /// Expands field codes after tokenizing, keeping replacements as argument data. + /// Shell scripts must be fixed; targets may only be supplied as positional arguments. /// public static IReadOnlyList> Expand(DesktopApplication application, IReadOnlyList targets) { - var exec = application.Exec; - if (string.IsNullOrWhiteSpace(exec)) - return []; + var tokens = Tokenize(application.Exec); + if (tokens is null || tokens.Count == 0 || tokens[0].Contains('%') || tokens[0].Contains('=')) return []; - var hasShell = (Tokenize(exec) ?? []).Any(t => Shells.Contains(System.IO.Path.GetFileName(t))); - var perTarget = HasCode(exec, 'f') || HasCode(exec, 'u'); - IReadOnlyList singles = !perTarget ? new string?[] { null } : targets.Count == 0 ? new string?[] { string.Empty } : [.. targets]; + // Quoted field codes have undefined semantics in the desktop entry specification. + var quote = '\0'; + for (var i = 0; i < application.Exec.Length; i++) + { + var c = application.Exec[i]; + if (c == '\\' && quote != '\'' && i + 1 < application.Exec.Length) { i++; continue; } + if (c == quote) quote = '\0'; + else if (quote == '\0' && c is '\'' or '"') quote = c; + else if (c == '%' && i + 1 < application.Exec.Length) + { + if (application.Exec[++i] != '%' && quote != '\0') return []; + } + } - var result = new List>(); - foreach (var single in singles) + var targetCodes = 0; + foreach (var token in tokens) { - var argv = Tokenize(Substitute(application, exec, targets, single, hasShell)); - if (argv is null || argv.Count == 0) - return []; + for (var i = 0; i < token.Length; i++) + { + if (token[i] != '%') continue; + if (++i == token.Length) return []; + var code = token[i]; + if (!"%fFuUickdDnNvm".Contains(code)) return []; + if (code is 'f' or 'F' or 'u' or 'U') targetCodes++; + if (code is 'F' or 'U' or 'i' && token.Length != 2) return []; + } + } + if (targetCodes > 1) return []; - result.Add(argv); + // Do not substitute into shell options or script text, including through env wrappers. + for (var i = 0; i < tokens.Count; i++) + { + if (System.IO.Path.GetFileName(tokens[i]) == "env" && tokens.Skip(i + 1).Any(t => t.StartsWith("-S", StringComparison.Ordinal) || t.StartsWith("--split-string", StringComparison.Ordinal))) return []; + if (!Shells.Contains(System.IO.Path.GetFileName(tokens[i]))) continue; + var script = -1; + for (var j = i + 1; j < tokens.Count; j++) + { + if (tokens[j].StartsWith('-') && !tokens[j].StartsWith("--", StringComparison.Ordinal) && tokens[j].Contains('c')) { script = j + 1; break; } + } + if (script < 0 || script >= tokens.Count) return []; + if (tokens.Take(script + 1).Any(t => t.Replace("%%", "", StringComparison.Ordinal).Contains('%'))) return []; + break; } + var perTarget = tokens.Any(t => HasCode(t, 'f') || HasCode(t, 'u')); + IReadOnlyList singles = !perTarget || targets.Count == 0 ? new string?[] { null } : [.. targets]; + var result = new List>(); + foreach (var single in singles) + { + var argv = new List(); + foreach (var token in tokens) + { + if (token == "%F") { argv.AddRange(targets.Select(PathArgument)); continue; } + if (token == "%U") { argv.AddRange(targets.Select(ToUri)); continue; } + if (token == "%i") + { + if (!string.IsNullOrEmpty(application.IconName)) { argv.Add("--icon"); argv.Add(application.IconName); } + continue; + } + var value = new StringBuilder(); + for (var i = 0; i < token.Length; i++) + { + if (token[i] != '%') { value.Append(token[i]); continue; } + value.Append(token[++i] switch + { + '%' => "%", + 'f' => single is null ? "" : PathArgument(single), + 'u' => single is null ? "" : ToUri(single), + 'c' => application.Name, + 'k' => application.DesktopFilePath, + _ => "", + }); + } + if (value.Length > 0 || token.Length == 0) argv.Add(value.ToString()); + } + if (argv.Count == 0 || argv.Any(a => a.Contains('\0'))) return []; + result.Add(argv.ToArray()); + } return result; } @@ -220,71 +281,6 @@ private static bool HasCode(string exec, char code) return false; } - private static string Substitute(DesktopApplication app, string exec, IReadOnlyList targets, string? single, bool hasShell) - { - var sb = new StringBuilder(); - var quote = '\0'; - - for (var i = 0; i < exec.Length; i++) - { - var c = exec[i]; - if (c != '%' || i + 1 >= exec.Length) - { - if (quote == '\0' && c is '"' or '\'') - quote = c; - else if (c == quote) - quote = '\0'; - else if (c == '\\' && quote != '\'' && i + 1 < exec.Length) - { - sb.Append(c).Append(exec[++i]); - continue; - } - - sb.Append(c); - continue; - } - - var code = exec[++i]; - var value = code switch - { - 'f' => string.IsNullOrEmpty(single) ? null : Quote(PathArgument(single), quote, hasShell), - 'u' => string.IsNullOrEmpty(single) ? null : Quote(ToUri(single), quote, hasShell), - 'F' => targets.Count == 0 ? null : string.Join(' ', targets.Select(t => Quote(PathArgument(t), quote, hasShell))), - 'U' => targets.Count == 0 ? null : string.Join(' ', targets.Select(t => Quote(ToUri(t), quote, hasShell))), - 'c' => Quote(app.Name, quote, hasShell), - 'k' => Quote(app.DesktopFilePath, quote, hasShell), - 'i' => string.IsNullOrEmpty(app.IconName) ? null : "--icon " + Quote(app.IconName, quote, hasShell), - _ => null, - }; - - if (code == '%') - sb.Append('%'); - else if (value is not null) - sb.Append(value); - } - - return sb.ToString(); - } - - /// - /// Quotes a value for insertion at the current quote context of the Exec string. - /// - private static string Quote(string value, char context, bool hasShell) - { - var quoted = ShellQuote(value); - if (context == '"' && !hasShell) - quoted = value; - return context switch - { - // Our tokenizer consumes one level of double-quote escapes, so escape for it - '"' => quoted.Replace("\\", "\\\\").Replace("\"", "\\\"").Replace("`", "\\`").Replace("$", "\\$"), - - // Shells: escape the quoted word for the raw single-quoted script. Others: close the region, insert the word, reopen - '\'' => hasShell ? quoted.Replace("'", "'\\''") : "'" + quoted + "'", - _ => quoted, - }; - } - /// /// Converts a target to a local path argument; relative paths starting with '-' are prefixed so they cannot be read as options. /// diff --git a/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs b/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs index 7ba813e29d62..00407500afb5 100644 --- a/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs +++ b/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs @@ -87,9 +87,9 @@ public void NoFieldCode_IgnoresTargets() } [TestMethod] - public void EmbeddedFieldCode_IsSubstituted() + public void QuotedFieldCode_IsRejected() { - CollectionAssert.AreEqual(new[] { "viewer", "--file=/x y" }, Expand("viewer \"--file=%f\"", null, "/x y")[0]); + Assert.AreEqual(0, Expand("viewer \"--file=%f\"", null, "/x y").Length); } [TestMethod] @@ -98,6 +98,24 @@ public void DeprecatedCodes_AreRemoved() CollectionAssert.AreEqual(new[] { "viewer" }, Expand("viewer %d %D %n %N %v %m")[0]); } + [TestMethod] + [DataRow("viewer %x")] + [DataRow("viewer %")] + [DataRow("viewer %f %U")] + [DataRow("viewer prefix%F")] + [DataRow("viewer --icon=%i")] + [DataRow("viewer \"%c\"")] + public void InvalidFieldCodes_AreRejected(string exec) + { + Assert.AreEqual(0, Expand(exec, null, "/x").Length); + } + + [TestMethod] + public void EmbeddedSingleCode_IsLiteralAndExpandedOnlyOnce() + { + CollectionAssert.AreEqual(new[] { "viewer", "--file=/x %U $(id)" }, Expand("viewer --file=%f", null, "/x %U $(id)")[0]); + } + [TestMethod] public void EmptyExec_YieldsNothing() { diff --git a/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs b/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs index 905461abad38..ffc432207847 100644 --- a/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs +++ b/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs @@ -5,6 +5,7 @@ using Files.Platform.Linux.Launching; using Microsoft.VisualStudio.TestTools.UnitTesting; using System.Linq; +using System.Diagnostics; namespace Files.Platform.Tests.Launching { @@ -51,46 +52,34 @@ public void MultipleFiles_AreSeparateLiteralArguments() } [TestMethod] - public void ShellDashC_ValueIsSingleQuotedInScript() + [DataRow("sh -c \"app %f\"")] + [DataRow("sh -c \"app \\\"%f\\\"\"")] + [DataRow("sh -c \"app '%f'\"")] + [DataRow("bash -c 'app %U; echo %c'")] + [DataRow("sh -c %f")] + [DataRow("env sh -lc %f")] + public void ShellScript_FieldCodesAreRejected(string exec) { foreach (var name in Nasty) - { - var argv = Expand("sh -c \"app %f\"", name)[0]; - Assert.AreEqual(3, argv.Length); - Assert.AreEqual("sh", argv[0]); - Assert.AreEqual("-c", argv[1]); - Assert.AreEqual("app " + DesktopExecExpander.ShellQuote(Safe(name)), argv[2]); - } - - Assert.AreEqual("app '/tmp/$(curl x|sh).txt'", Expand("sh -c \"app %f\"", Nasty[0])[0][2]); - Assert.AreEqual("app '/tmp/it'\\''s \"quoted\".txt'", Expand("sh -c \"app %f\"", Nasty[3])[0][2]); + Assert.AreEqual(0, Expand(exec, name).Length, exec); } [TestMethod] - public void ShellDashC_MultipleFiles_AreEachQuoted() + public void FixedShellScript_TargetsAreLiteralPositionalArguments() { - var argv = Expand("bash -c \"app %F\"", "/a b", "/c'd")[0]; - - Assert.AreEqual("app '/a b' '/c'\\''d'", argv[2]); - } - - [TestMethod] - public void ShellSingleQuotedScript_AndUriCodes() - { - var argv = Expand("bash -c 'app %U; echo %c'", "/tmp/$(x).txt")[0]; - - Assert.AreEqual("app 'file:///tmp/$(x).txt'; echo 'Test App'", argv[2]); - Assert.AreEqual("app '/it'\\''s'", Expand("bash -c 'app %f'", "/it's")[0][2]); - } - - [TestMethod] - public void NameAndDesktopPath_AreQuoted() - { - var app = new DesktopApplication("t.desktop", "Evil $(x) 'name'", "sh -c \"echo %c %k\"", "/a b/t.desktop"); - - var argv = DesktopExecExpander.Expand(app, [])[0]; + foreach (var name in Nasty) + { + var argv = Expand("sh -c \"printf '%%s' \\\"\\$1\\\"\" sh %f", name)[0]; + CollectionAssert.AreEqual(new[] { "sh", "-c", "printf '%s' \"$1\"", "sh", Safe(name) }, argv); + var start = new ProcessStartInfo("/bin/sh") { UseShellExecute = false, RedirectStandardOutput = true }; + foreach (var argument in argv.Skip(1)) start.ArgumentList.Add(argument); + using var process = Process.Start(start)!; + var output = process.StandardOutput.ReadToEnd(); + process.WaitForExit(); + Assert.AreEqual(0, process.ExitCode); + Assert.AreEqual(Safe(name), output); - Assert.AreEqual("echo 'Evil $(x) '\\''name'\\''' '/a b/t.desktop'", argv[2]); + } } [TestMethod] diff --git a/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs b/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs index 13b89e74aa4b..fc375e26a9aa 100644 --- a/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs +++ b/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs @@ -246,6 +246,16 @@ public async Task Open_StartFailure_ReturnsFalse() Assert.IsFalse(await svc.OpenAsync(["/a.txt"])); } + [TestMethod] + public async Task OpenWith_ShellScriptFieldCodeDoesNotStartProcess() + { + using var fx = new XdgFixture(); + var (service, starter) = Create(fx); + var app = new Files.Platform.Abstractions.Mime.DesktopApplication("t.desktop", "Test", "sh -c \"viewer \\\"%f\\\"\"", "/t.desktop"); + Assert.IsFalse(await service.OpenWithAsync(app, ["/tmp/$(printf INJECTED).txt"])); + Assert.AreEqual(0, starter.Launches.Count); + } + [TestMethod] public async Task OpenWith_TerminalApp_IsWrappedInTerminal() { From 205d09fbd8a72fbfec93101ba2e1c1f330b324ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Mon, 12 Oct 2026 23:58:33 +0300 Subject: [PATCH 2/4] Address review findings Co-Authored-By: Codex Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../Launching/DesktopExecExpander.cs | 36 ++++++++++++++----- .../Launching/DesktopExecInjectionTests.cs | 35 ++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs index 9ec0765d4e77..ec31709bd75b 100644 --- a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs +++ b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs @@ -14,7 +14,7 @@ namespace Files.Platform.Linux.Launching /// public static class DesktopExecExpander { - private static readonly HashSet Shells = ["sh", "bash", "dash", "zsh", "ksh", "fish", "ash", "csh", "tcsh"]; + private static readonly HashSet Shells = ["sh", "bash", "dash", "zsh", "ksh", "fish", "ash", "csh", "tcsh", "rbash"]; /// /// Splits an Exec value into arguments using shell quoting rules (double quotes, single quotes, backslash). @@ -117,19 +117,31 @@ public static IReadOnlyList> Expand(DesktopApplication app } if (targetCodes > 1) return []; - // Do not substitute into shell options or script text, including through env wrappers. + // Inspect every interpreter in wrapper chains, but not assignments or shell argv[0]. + var shellNames = new HashSet(); for (var i = 0; i < tokens.Count; i++) { - if (System.IO.Path.GetFileName(tokens[i]) == "env" && tokens.Skip(i + 1).Any(t => t.StartsWith("-S", StringComparison.Ordinal) || t.StartsWith("--split-string", StringComparison.Ordinal))) return []; - if (!Shells.Contains(System.IO.Path.GetFileName(tokens[i]))) continue; + if (tokens[i].Contains('=')) continue; + if (shellNames.Contains(i) && (i + 1 == tokens.Count || !tokens[i + 1].StartsWith('-'))) continue; + var executable = ExecutableName(tokens[i]); + if (executable == "env" && tokens.Skip(i + 1).Any(t => t.StartsWith("-S", StringComparison.Ordinal) || t.StartsWith("--split-string", StringComparison.Ordinal))) return []; + if (!Shells.Contains(executable)) continue; var script = -1; + var commandString = false; for (var j = i + 1; j < tokens.Count; j++) { - if (tokens[j].StartsWith('-') && !tokens[j].StartsWith("--", StringComparison.Ordinal) && tokens[j].Contains('c')) { script = j + 1; break; } + if (tokens[j] == "--") { script = j + 1; break; } + if (!tokens[j].StartsWith('-') || tokens[j] == "-") { script = j; break; } + if (!tokens[j].StartsWith("--", StringComparison.Ordinal) && tokens[j].Contains('c')) + { + script = j + 1; + commandString = true; + break; + } } - if (script < 0 || script >= tokens.Count) return []; - if (tokens.Take(script + 1).Any(t => t.Replace("%%", "", StringComparison.Ordinal).Contains('%'))) return []; - break; + if (script < 0 || script >= tokens.Count) continue; + if (tokens.Skip(i).Take(script - i + 1).Any(t => t.Replace("%%", "", StringComparison.Ordinal).Contains('%'))) return []; + if (commandString) shellNames.Add(script + 1); } var perTarget = tokens.Any(t => HasCode(t, 'f') || HasCode(t, 'u')); @@ -226,6 +238,14 @@ public static IReadOnlyList> ExpandServiceMenu(DesktopAppl return result; } + private static string ExecutableName(string token) + { + var name = System.IO.Path.GetFileName(token); + if (Shells.Contains(name)) return name; + var path = new PathExecutableLocator().Locate(token); + return path is null ? name : System.IO.Path.GetFileName(Trash.LinuxTrashService.RealPath(path)); + } + // Accept only --name=%x with an ASCII option name and no surrounding value text. private static char LongOptionTargetCode(string token) { diff --git a/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs b/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs index ffc432207847..cd384dd5d1b8 100644 --- a/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs +++ b/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs @@ -82,6 +82,41 @@ public void FixedShellScript_TargetsAreLiteralPositionalArguments() } } + [TestMethod] + [DataRow("sh -c 'exec \"$@\"' sh bash -c %f")] + [DataRow("env sh -c 'exec \"$@\"' sh env bash -c %f")] + [DataRow("sh -c 'exec \"$0\" \"$@\"' bash -c %f")] + [DataRow("rbash -c %f")] + [DataRow("bash %f")] + public void NestedShellsAndScriptFiles_RejectTargetScripts(string exec) + { + Assert.AreEqual(0, Expand(exec, "/nonexistent/$(printf BYPASS >&2).txt").Length); + } + + [TestMethod] + public void ShellSymlink_RejectsTargetScript() + { + var directory = System.IO.Path.Combine(System.IO.Path.GetTempPath(), System.Guid.NewGuid().ToString()); + System.IO.Directory.CreateDirectory(directory); + try + { + var alias = System.IO.Path.Combine(directory, "launcher"); + System.IO.File.CreateSymbolicLink(alias, "/bin/bash"); + Assert.AreEqual(0, Expand(alias + " -c %f", "/tmp/$(id).txt").Length); + Assert.AreEqual(0, Expand("env " + alias + " %f", "/tmp/$(id).txt").Length); + } + finally { System.IO.Directory.Delete(directory, true); } + } + + [TestMethod] + [DataRow("env SHELL=/bin/bash viewer %f")] + [DataRow("env VAR=bash viewer %f")] + [DataRow("viewer bash")] + public void OrdinaryShellNamedArguments_AreAccepted(string exec) + { + Assert.AreEqual(1, Expand(exec, "/tmp/file.txt").Length); + } + [TestMethod] public void UnbalancedQuotes_AreRejected() { From 499282c83bec43e5e030eb548ffff5ec57082ddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Tue, 13 Oct 2026 00:50:46 +0300 Subject: [PATCH 3/4] Expand desktop Exec field codes only as separate argv entries Co-Authored-By: Codex Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../investigations/security-review-2026-10.md | 2 +- .../Launching/DesktopExecExpander.cs | 277 +++--------------- .../Launching/IProcessStarter.cs | 26 +- .../Launching/DesktopExecExpanderTests.cs | 32 +- .../Launching/DesktopExecInjectionTests.cs | 51 +--- .../Launching/LauncherServiceTests.cs | 2 +- .../Launching/OpenDecisionTests.cs | 2 +- .../Mime/ServiceMenuTests.cs | 41 +-- 8 files changed, 98 insertions(+), 335 deletions(-) diff --git a/docs/linux-port/investigations/security-review-2026-10.md b/docs/linux-port/investigations/security-review-2026-10.md index 02e5ff16468b..d53b0fcfb3a0 100644 --- a/docs/linux-port/investigations/security-review-2026-10.md +++ b/docs/linux-port/investigations/security-review-2026-10.md @@ -14,7 +14,7 @@ Priorities: P0 critical immediate exposure; P1 command execution or significant **Evidence:** Reproduced with `/bin/sh` and harmless `printf INJECTED`. The expanded script contained the literal `$(printf INJECTED)` filename; shell output instead contained `INJECTED`. Existing `DesktopExecInjectionTests` cover unquoted placeholders inside scripts, not this nested case. Service-menu expansion uses a separate restrictive path; this finding concerns `Expand`. -**Suggested fix:** Tokenize the desktop Exec before substituting filenames and keep targets as argv data. Reject field-code substitution into shell script arguments; support a safe explicitly defined form passing targets as positional arguments to a fixed script (`sh -c 'viewer "$1"' sh `). Do not attempt to fix arbitrary nested script syntax with another escaping pass. Add nested double/single quotes, substitutions and backticks to launch tests. +**Resolution:** Parse Exec using Desktop Entry quoting rules and reject field codes embedded in or quoted within arguments. Expand codes only into separate argv entries; `%%` represents a literal percent. Remove shell/wrapper detection entirely. Exec lines from installed desktop entries and service menus are trusted configuration (the same trust model as GIO); filenames are untrusted data and are only ever passed as separate argv. A configured interpreter can intentionally interpret an argument as code; identifying or policing such trusted commands is outside this boundary. LinuxFiles must not evaluate Exec through a shell and uses `ProcessStartInfo.ArgumentList`. ### R2 — P1: Archive merge can delete/overwrite outside the extraction destination diff --git a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs index ec31709bd75b..de7291cd1fcb 100644 --- a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs +++ b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs @@ -14,254 +14,89 @@ namespace Files.Platform.Linux.Launching /// public static class DesktopExecExpander { - private static readonly HashSet Shells = ["sh", "bash", "dash", "zsh", "ksh", "fish", "ash", "csh", "tcsh", "rbash"]; - /// - /// Splits an Exec value into arguments using shell quoting rules (double quotes, single quotes, backslash). - /// Returns null when quotes are unbalanced. + /// Parses an Exec value after desktop-entry string unescaping. Only whole double-quoted arguments are supported. /// public static List? Tokenize(string exec) { var tokens = new List(); - var current = new StringBuilder(); - var hasToken = false; - var quote = '\0'; - - for (var i = 0; i < exec.Length; i++) + for (var i = 0; i < exec.Length;) { - var c = exec[i]; - if (quote == '\'') - { - if (c == '\'') - quote = '\0'; - else - current.Append(c); - } - else if (quote == '"') - { - if (c == '\\' && i + 1 < exec.Length && exec[i + 1] is '"' or '`' or '$' or '\\') - current.Append(exec[++i]); - else if (c == '"') - quote = '\0'; - else - current.Append(c); - } - else if (c is '"' or '\'') - { - quote = c; - hasToken = true; - } - else if (c == '\\' && i + 1 < exec.Length) - { - current.Append(exec[++i]); - hasToken = true; - } - else if (c is ' ' or '\t') - { - if (hasToken) - tokens.Add(current.ToString()); - current.Clear(); - hasToken = false; - } - else + if (exec[i] is ' ' or '\t') { i++; continue; } + var quoted = exec[i] == '"'; + if (quoted) i++; + var value = new StringBuilder(); + var closed = !quoted; + while (i < exec.Length) { - current.Append(c); - hasToken = true; + var c = exec[i++]; + if (quoted && c == '"') { closed = true; break; } + if (!quoted && c is ' ' or '\t') break; + if (quoted && c == '\\') + { + if (i == exec.Length || exec[i] is not ('"' or '`' or '$' or '\\')) return null; + c = exec[i++]; + } + else if ((!quoted && "\"'\\><~|&;$*?#()`\n\r".Contains(c)) || + (quoted && c is '$' or '`')) return null; + if (c == '%') + { + if (i == exec.Length) return null; + var code = exec[i++]; + if (code != '%' && (quoted || !"fFuUickdDnNvm".Contains(code))) return null; + value.Append('%').Append(code); + } + else value.Append(c); } + if (!closed || (quoted && i < exec.Length && exec[i] is not (' ' or '\t'))) return null; + var token = value.ToString(); + for (var j = 0; j < token.Length; j++) + if (token[j] == '%' && token[++j] != '%' && token.Length != 2) return null; + tokens.Add(token); } - - if (quote != '\0') - return null; - - if (hasToken) - tokens.Add(current.ToString()); - return tokens; } - /// - /// Expands field codes after tokenizing, keeping replacements as argument data. - /// Shell scripts must be fixed; targets may only be supplied as positional arguments. - /// + /// Expands field codes into separate argv entries, never command text. public static IReadOnlyList> Expand(DesktopApplication application, IReadOnlyList targets) { + // Installed desktop entries/service menus are trusted configuration, as with GIO. + // Filenames are untrusted data: substitute only separate argv entries, never shell text. var tokens = Tokenize(application.Exec); - if (tokens is null || tokens.Count == 0 || tokens[0].Contains('%') || tokens[0].Contains('=')) return []; - - // Quoted field codes have undefined semantics in the desktop entry specification. - var quote = '\0'; - for (var i = 0; i < application.Exec.Length; i++) - { - var c = application.Exec[i]; - if (c == '\\' && quote != '\'' && i + 1 < application.Exec.Length) { i++; continue; } - if (c == quote) quote = '\0'; - else if (quote == '\0' && c is '\'' or '"') quote = c; - else if (c == '%' && i + 1 < application.Exec.Length) - { - if (application.Exec[++i] != '%' && quote != '\0') return []; - } - } - - var targetCodes = 0; - foreach (var token in tokens) - { - for (var i = 0; i < token.Length; i++) - { - if (token[i] != '%') continue; - if (++i == token.Length) return []; - var code = token[i]; - if (!"%fFuUickdDnNvm".Contains(code)) return []; - if (code is 'f' or 'F' or 'u' or 'U') targetCodes++; - if (code is 'F' or 'U' or 'i' && token.Length != 2) return []; - } - } - if (targetCodes > 1) return []; - - // Inspect every interpreter in wrapper chains, but not assignments or shell argv[0]. - var shellNames = new HashSet(); - for (var i = 0; i < tokens.Count; i++) - { - if (tokens[i].Contains('=')) continue; - if (shellNames.Contains(i) && (i + 1 == tokens.Count || !tokens[i + 1].StartsWith('-'))) continue; - var executable = ExecutableName(tokens[i]); - if (executable == "env" && tokens.Skip(i + 1).Any(t => t.StartsWith("-S", StringComparison.Ordinal) || t.StartsWith("--split-string", StringComparison.Ordinal))) return []; - if (!Shells.Contains(executable)) continue; - var script = -1; - var commandString = false; - for (var j = i + 1; j < tokens.Count; j++) - { - if (tokens[j] == "--") { script = j + 1; break; } - if (!tokens[j].StartsWith('-') || tokens[j] == "-") { script = j; break; } - if (!tokens[j].StartsWith("--", StringComparison.Ordinal) && tokens[j].Contains('c')) - { - script = j + 1; - commandString = true; - break; - } - } - if (script < 0 || script >= tokens.Count) continue; - if (tokens.Skip(i).Take(script - i + 1).Any(t => t.Replace("%%", "", StringComparison.Ordinal).Contains('%'))) return []; - if (commandString) shellNames.Add(script + 1); - } - - var perTarget = tokens.Any(t => HasCode(t, 'f') || HasCode(t, 'u')); + if (tokens is null || tokens.Count == 0 || tokens[0].Length == 0 || + tokens[0].Contains('=') || tokens[0].Replace("%%", "", StringComparison.Ordinal).Contains('%')) return []; + if (tokens.Count(t => t is "%f" or "%F" or "%u" or "%U") > 1) return []; + var perTarget = tokens.Any(t => t is "%f" or "%u"); IReadOnlyList singles = !perTarget || targets.Count == 0 ? new string?[] { null } : [.. targets]; var result = new List>(); foreach (var single in singles) - { - var argv = new List(); - foreach (var token in tokens) - { - if (token == "%F") { argv.AddRange(targets.Select(PathArgument)); continue; } - if (token == "%U") { argv.AddRange(targets.Select(ToUri)); continue; } - if (token == "%i") - { - if (!string.IsNullOrEmpty(application.IconName)) { argv.Add("--icon"); argv.Add(application.IconName); } - continue; - } - var value = new StringBuilder(); - for (var i = 0; i < token.Length; i++) - { - if (token[i] != '%') { value.Append(token[i]); continue; } - value.Append(token[++i] switch - { - '%' => "%", - 'f' => single is null ? "" : PathArgument(single), - 'u' => single is null ? "" : ToUri(single), - 'c' => application.Name, - 'k' => application.DesktopFilePath, - _ => "", - }); - } - if (value.Length > 0 || token.Length == 0) argv.Add(value.ToString()); - } - if (argv.Count == 0 || argv.Any(a => a.Contains('\0'))) return []; - result.Add(argv.ToArray()); - } - return result; - } - - /// Service menus never invoke a shell. Only a single target may fill a long option's entire value. - public static IReadOnlyList> ExpandServiceMenu(DesktopApplication app, IReadOnlyList targets) - { - // Refuse shell syntax conservatively, even if quoted. Substituted paths are not inspected as command text. - if (app.Exec.Any(c => c is '|' or '&' or ';' or '<' or '>' or '`' or '$' or '*' or '?' or '~' or '#' || char.IsControl(c))) return []; - var tokens = Tokenize(app.Exec); - if (tokens is null || tokens.Count == 0 || tokens[0].Contains('%') || tokens[0].Contains('=') || - tokens.Any(t => Shells.Contains(System.IO.Path.GetFileName(t))) || - (System.IO.Path.GetFileName(tokens[0]) == "env" && tokens.Any(t => t.StartsWith("-S", StringComparison.Ordinal) || t.StartsWith("--split-string", StringComparison.Ordinal)))) return []; - var targetCodes = tokens.Select(t => t is "%f" or "%F" or "%u" or "%U" ? t[1] : LongOptionTargetCode(t)).ToArray(); - if (targetCodes.Count(c => c != '\0') > 1 || - (targetCodes.Any(c => c is 'f' or 'F' or 'd' or 'D') && targets.Any(t => !ToPathOrUri(t).StartsWith('/'))) || - (tokens.Any(t => LongOptionTargetCode(t) != '\0') && targets.Count != 1)) return []; - var single = tokens.Any(t => t is "%f" or "%u"); - var result = new List>(); - foreach (var target in single ? targets : new string[] { string.Empty }) { var argv = new List(); foreach (var token in tokens) { switch (token) { - case "%f": argv.Add(PathArgument(target)); break; - case "%u": argv.Add(ToUri(target)); break; + case "%f": if (single is not null) argv.Add(PathArgument(single)); break; + case "%u": if (single is not null) argv.Add(ToUri(single)); break; case "%F": argv.AddRange(targets.Select(PathArgument)); break; case "%U": argv.AddRange(targets.Select(ToUri)); break; - case "%c": argv.Add(app.Name); break; - case "%k": argv.Add(app.DesktopFilePath); break; + case "%c": argv.Add(application.Name); break; + case "%k": argv.Add(application.DesktopFilePath); break; case "%i": - if (!string.IsNullOrEmpty(app.IconName)) { argv.Add("--icon"); argv.Add(app.IconName); } - break; - default: - var code = LongOptionTargetCode(token); - if (code != '\0') - { - var value = code is 'u' or 'U' ? ToUri(targets[0]) : PathArgument(targets[0]); - if (code is 'd' or 'D') value = System.IO.Path.GetDirectoryName(value) ?? "/"; - argv.Add(token[..^2] + value); - break; - } - var literal = new StringBuilder(); - for (var i = 0; i < token.Length; i++) - { - if (token[i] != '%') literal.Append(token[i]); - else if (i + 1 < token.Length && token[++i] == '%') literal.Append('%'); - else return []; - } - argv.Add(literal.ToString()); + if (!string.IsNullOrEmpty(application.IconName)) { argv.Add("--icon"); argv.Add(application.IconName); } break; + case "%d": case "%D": case "%n": case "%N": case "%v": case "%m": break; + default: argv.Add(token.Replace("%%", "%", StringComparison.Ordinal)); break; } } - if (argv.Count == 0 || argv.Any(a => a.Contains('\0'))) return []; + if (argv.Any(a => a.Contains('\0'))) return []; result.Add(argv.ToArray()); } return result; } - private static string ExecutableName(string token) - { - var name = System.IO.Path.GetFileName(token); - if (Shells.Contains(name)) return name; - var path = new PathExecutableLocator().Locate(token); - return path is null ? name : System.IO.Path.GetFileName(Trash.LinuxTrashService.RealPath(path)); - } - - // Accept only --name=%x with an ASCII option name and no surrounding value text. - private static char LongOptionTargetCode(string token) - { - var equals = token.IndexOf('='); - if (!token.StartsWith("--", StringComparison.Ordinal) || equals < 3 || equals != token.Length - 3 || - token[equals + 1] != '%' || token[^1] is not ('f' or 'F' or 'u' or 'U' or 'd' or 'D') || - !char.IsAsciiLetterOrDigit(token[2])) return '\0'; - for (var i = 3; i < equals; i++) - if (!char.IsAsciiLetterOrDigit(token[i]) && token[i] != '-') return '\0'; - return token[^1]; - } - - /// - /// Quotes a value as a single shell word: wrapped in single quotes with embedded quotes escaped. - /// - public static string ShellQuote(string value) => "'" + value.Replace("'", "'\\''") + "'"; + /// Service menus use the same Exec grammar and argv expansion as desktop entries. + public static IReadOnlyList> ExpandServiceMenu(DesktopApplication app, IReadOnlyList targets) => Expand(app, targets); /// /// Converts a path or file URI to a local path; other URIs are returned unchanged. @@ -285,22 +120,6 @@ public static string ToUri(string target) return new Uri(target).AbsoluteUri; } - private static bool HasCode(string exec, char code) - { - for (var i = 0; i + 1 < exec.Length; i++) - { - if (exec[i] != '%') - continue; - - if (exec[i + 1] == code) - return true; - - i++; - } - - return false; - } - /// /// Converts a target to a local path argument; relative paths starting with '-' are prefixed so they cannot be read as options. /// diff --git a/src/Files.Platform.Linux/Launching/IProcessStarter.cs b/src/Files.Platform.Linux/Launching/IProcessStarter.cs index dac841ef699a..63891b2dd430 100644 --- a/src/Files.Platform.Linux/Launching/IProcessStarter.cs +++ b/src/Files.Platform.Linux/Launching/IProcessStarter.cs @@ -30,13 +30,10 @@ public interface IProcessStarter } /// - /// Starts processes through setsid with stdio redirected to /dev/null so they outlive and ignore this process. + /// Starts processes through setsid when available, passing arguments directly without a shell. /// public sealed class DetachedProcessStarter : IProcessStarter { - private const string RedirectScript = "exec \"$@\" /dev/null 2>&1"; - private const string BackgroundScript = "exec \"$@\" /dev/null 2>&1 &"; - private readonly IExecutableLocator locator; /// @@ -55,20 +52,16 @@ public DetachedProcessStarter(IExecutableLocator locator) } /// - public async Task StartDetachedAsync(ProcessLaunch launch, CancellationToken cancellationToken = default) + public Task StartDetachedAsync(ProcessLaunch launch, CancellationToken cancellationToken = default) { var program = locator.Locate(launch.FileName) ?? throw new FileNotFoundException("Executable not found.", launch.FileName); - var sh = locator.Locate("sh") ?? "/bin/sh"; var setsid = locator.Locate("setsid"); var startInfo = new ProcessStartInfo { UseShellExecute = false, - RedirectStandardInput = true, - RedirectStandardOutput = true, - RedirectStandardError = true, CreateNoWindow = true, }; @@ -79,29 +72,20 @@ public async Task StartDetachedAsync(ProcessLaunch launch, CancellationToken can { startInfo.FileName = setsid; startInfo.ArgumentList.Add("--fork"); - startInfo.ArgumentList.Add(sh); + startInfo.ArgumentList.Add(program); } else { - startInfo.FileName = sh; + startInfo.FileName = program; } - startInfo.ArgumentList.Add("-c"); - startInfo.ArgumentList.Add(setsid is not null ? RedirectScript : BackgroundScript); - startInfo.ArgumentList.Add("sh"); - startInfo.ArgumentList.Add(program); foreach (var argument in launch.Arguments) startInfo.ArgumentList.Add(argument); using var process = Files.Platform.Linux.Launching.TracedProcess.Start(startInfo) ?? throw new InvalidOperationException("The process could not be started."); - process.StandardInput.Close(); - process.StandardOutput.Close(); - process.StandardError.Close(); - - using var trace = Files.Platform.Abstractions.Diagnostics.PerformanceTrace.Begin("process-wrapper-exit"); - await process.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + return Task.CompletedTask; } } diff --git a/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs b/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs index 00407500afb5..6350c8f73193 100644 --- a/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs +++ b/tests/Files.Platform.Tests/Launching/DesktopExecExpanderTests.cs @@ -22,7 +22,26 @@ public void Tokenize_HandlesQuotesAndEscapes() { CollectionAssert.AreEqual( new[] { "my app", "--arg=a b", "say \"hi\"", "$HOME", "" }, - DesktopExecExpander.Tokenize("\"my app\" --arg=\"a b\" \"say \\\"hi\\\"\" \"\\$HOME\" \"\"")); + DesktopExecExpander.Tokenize("\"my app\" \"--arg=a b\" \"say \\\"hi\\\"\" \"\\$HOME\" \"\"")); + } + + [TestMethod] + [DataRow("viewer 'single quotes'")] + [DataRow("viewer prefix\"quoted\"")] + [DataRow("viewer \"quoted\"suffix")] + [DataRow("viewer $(id)")] + [DataRow("viewer a;b")] + [DataRow("viewer a\\b")] + [DataRow("viewer \"$HOME\"")] + public void InvalidExecQuoting_IsRejected(string exec) + { + Assert.IsNull(DesktopExecExpander.Tokenize(exec)); + } + + [TestMethod] + public void LiteralPercent_IsUnescapedOnlyOnce() + { + CollectionAssert.AreEqual(new[] { "viewer", "%f", "100%", "" }, Expand("viewer %%f \"100%%\" \"\"")[0]); } [TestMethod] @@ -55,10 +74,10 @@ public void UppercaseU_ConvertsPathsToUris() [TestMethod] public void IconNameAndKeyCodes() { - var result = Expand("viewer %i --title=%c --desktop=%k 100%% %f", "my-icon", "/x"); + var result = Expand("viewer %i --title %c --desktop %k 100%% %f", "my-icon", "/x"); CollectionAssert.AreEqual( - new[] { "viewer", "--icon", "my-icon", "--title=Test App", "--desktop=/usr/share/applications/t.desktop", "100%", "/x" }, + new[] { "viewer", "--icon", "my-icon", "--title", "Test App", "--desktop", "/usr/share/applications/t.desktop", "100%", "/x" }, result[0]); } @@ -99,6 +118,9 @@ public void DeprecatedCodes_AreRemoved() } [TestMethod] + [DataRow("viewer --file=%f")] + [DataRow("viewer prefix%c")] + [DataRow("viewer \"%f\"")] [DataRow("viewer %x")] [DataRow("viewer %")] [DataRow("viewer %f %U")] @@ -111,9 +133,9 @@ public void InvalidFieldCodes_AreRejected(string exec) } [TestMethod] - public void EmbeddedSingleCode_IsLiteralAndExpandedOnlyOnce() + public void SingleCode_IsExpandedOnlyOnce() { - CollectionAssert.AreEqual(new[] { "viewer", "--file=/x %U $(id)" }, Expand("viewer --file=%f", null, "/x %U $(id)")[0]); + CollectionAssert.AreEqual(new[] { "viewer", "/x %U $(id)" }, Expand("viewer %f", null, "/x %U $(id)")[0]); } [TestMethod] diff --git a/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs b/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs index cd384dd5d1b8..552b0373e8ab 100644 --- a/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs +++ b/tests/Files.Platform.Tests/Launching/DesktopExecInjectionTests.cs @@ -51,19 +51,6 @@ public void MultipleFiles_AreSeparateLiteralArguments() Assert.AreEqual(Safe(Nasty[i]), argv[i + 1]); } - [TestMethod] - [DataRow("sh -c \"app %f\"")] - [DataRow("sh -c \"app \\\"%f\\\"\"")] - [DataRow("sh -c \"app '%f'\"")] - [DataRow("bash -c 'app %U; echo %c'")] - [DataRow("sh -c %f")] - [DataRow("env sh -lc %f")] - public void ShellScript_FieldCodesAreRejected(string exec) - { - foreach (var name in Nasty) - Assert.AreEqual(0, Expand(exec, name).Length, exec); - } - [TestMethod] public void FixedShellScript_TargetsAreLiteralPositionalArguments() { @@ -83,38 +70,14 @@ public void FixedShellScript_TargetsAreLiteralPositionalArguments() } [TestMethod] - [DataRow("sh -c 'exec \"$@\"' sh bash -c %f")] - [DataRow("env sh -c 'exec \"$@\"' sh env bash -c %f")] - [DataRow("sh -c 'exec \"$0\" \"$@\"' bash -c %f")] - [DataRow("rbash -c %f")] - [DataRow("bash %f")] - public void NestedShellsAndScriptFiles_RejectTargetScripts(string exec) - { - Assert.AreEqual(0, Expand(exec, "/nonexistent/$(printf BYPASS >&2).txt").Length); - } - - [TestMethod] - public void ShellSymlink_RejectsTargetScript() - { - var directory = System.IO.Path.Combine(System.IO.Path.GetTempPath(), System.Guid.NewGuid().ToString()); - System.IO.Directory.CreateDirectory(directory); - try - { - var alias = System.IO.Path.Combine(directory, "launcher"); - System.IO.File.CreateSymbolicLink(alias, "/bin/bash"); - Assert.AreEqual(0, Expand(alias + " -c %f", "/tmp/$(id).txt").Length); - Assert.AreEqual(0, Expand("env " + alias + " %f", "/tmp/$(id).txt").Length); - } - finally { System.IO.Directory.Delete(directory, true); } - } - - [TestMethod] - [DataRow("env SHELL=/bin/bash viewer %f")] - [DataRow("env VAR=bash viewer %f")] - [DataRow("viewer bash")] - public void OrdinaryShellNamedArguments_AreAccepted(string exec) + [DataRow("sh -c %f")] + [DataRow("env -S %f")] + [DataRow("nice bash --rcfile %f")] + [DataRow("launcher %f")] + public void TrustedCommands_AreNotInspected(string exec) { - Assert.AreEqual(1, Expand(exec, "/tmp/file.txt").Length); + foreach (var name in Nasty) + Assert.AreEqual(Safe(name), Expand(exec, name)[0][^1]); } [TestMethod] diff --git a/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs b/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs index fc375e26a9aa..fde661c4c47b 100644 --- a/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs +++ b/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs @@ -154,7 +154,7 @@ public async Task RunCommand_StartsExactlyTheDisplayedArgv_PlainAndInTerminal() { using var fx = new XdgFixture(); var (svc, starter) = Create(fx, new Dictionary { ["TERMINAL"] = "xterm" }, "xterm"); - var app = new Files.Platform.Abstractions.Mime.DesktopApplication("x.desktop", "X", "/bin/echo \"a b\" \\\\s 'c d' %c", "/x.desktop"); + var app = new Files.Platform.Abstractions.Mime.DesktopApplication("x.desktop", "X", "/bin/echo \"a b\" \"c d\" %c", "/x.desktop"); var argv = DesktopExecExpander.Expand(app, [])[0]; Assert.IsTrue(await svc.RunCommandAsync(argv, false)); diff --git a/tests/Files.Platform.Tests/Launching/OpenDecisionTests.cs b/tests/Files.Platform.Tests/Launching/OpenDecisionTests.cs index 1279e07e4c3c..34b1820543e2 100644 --- a/tests/Files.Platform.Tests/Launching/OpenDecisionTests.cs +++ b/tests/Files.Platform.Tests/Launching/OpenDecisionTests.cs @@ -141,7 +141,7 @@ public void StrictParse_RejectsLocalizedBehaviorKeys(string text) [TestMethod] public void Exec_ParsedOnceMeansDisplayedArgvIsTheLaunchedArgv() { - var entry = Strict("[Desktop Entry]\nType=Application\nName=X\nExec=/bin/echo \"a\\\\\\\\ b\" 'c d' \\\\s %c\n", out var error)!; + var entry = Strict("[Desktop Entry]\nType=Application\nName=X\nExec=/bin/echo \"a b\" \"c d\" %c\n", out var error)!; Assert.IsNull(error); var first = Files.Platform.Linux.Launching.DesktopExecExpander.Expand(entry.Application, [])[0]; diff --git a/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs b/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs index a5489484af0b..a42b2fbe50fc 100644 --- a/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs +++ b/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Files Community +// Copyright (c) Files Community // Licensed under the MIT License. using Files.Platform.Abstractions.Mime; @@ -311,7 +311,7 @@ public void Plan_CapsPerTargetLaunchesWithoutLimitingBatchActions(string field, public void Expansion_TargetsRemainLiteralArgvElements(string field, bool urls, bool single) { string[] targets = ["/tmp/a 'quote' \"double\" $(touch bad);\nfile.png", "/tmp/-b\\file.png"]; - var app = new DesktopApplication("run", "Run", "tool \"" + field + "\"", "/menu.desktop"); + var app = new DesktopApplication("run", "Run", "tool " + field, "/menu.desktop"); var commands = DesktopExecExpander.ExpandServiceMenu(app, targets); Assert.AreEqual(single ? 2 : 1, commands.Count); var expected = urls ? targets.Select(DesktopExecExpander.ToUri).ToArray() : targets; @@ -339,7 +339,6 @@ public void Expansion_TargetsRemainLiteralArgvElements(string field, bool urls, [DataRow("tool --input=%f %U")] [DataRow("tool --input=%f --other=%f")] [DataRow("tool '%F suffix'")] - [DataRow("tool %D")] [DataRow("tool 'unterminated")] public void Expansion_RefusesShellsSyntaxAndEmbeddedTargetCodes(string exec) => Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(new DesktopApplication("run", "Run", exec, "/menu.desktop"), ["/a.png"]).Count); @@ -351,18 +350,10 @@ public void Expansion_RefusesShellsSyntaxAndEmbeddedTargetCodes(string exec) => [DataRow("U")] [DataRow("d")] [DataRow("D")] - public void Expansion_LongOptionValueUsesExactlyOneLiteralTarget(string code) + public void Expansion_LongOptionValueRejectsEmbeddedCodes(string code) { - var target = "/tmp/a 'quoted' $(touch bad); directory/file.png"; var app = new DesktopApplication("run", "Run", "tool --long-option=%" + code, "/any-name.desktop"); - var commands = DesktopExecExpander.ExpandServiceMenu(app, [target]); - var value = code is "u" or "U" ? DesktopExecExpander.ToUri(target) : code is "d" or "D" ? Path.GetDirectoryName(target) : target; - CollectionAssert.AreEqual(new[] { "tool", "--long-option=" + value }, commands.Single().ToArray()); - Assert.IsNotNull(DisplaySanitizer.FullArguments(commands[0])); - Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(app, []).Count); - Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(app, [target, "/another"]).Count); - if (code is "f" or "F" or "d" or "D") - Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(app, ["smb://host/file"]).Count); + Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(app, ["/tmp/file"]).Count); } [TestMethod] @@ -373,7 +364,7 @@ public void Plan_RefusesArgvThatCannotBeDisplayedInFull() var action = ServiceMenuParser.ParseStrict(Text().Split('\n'), path, CultureInfo.InvariantCulture)!.Actions[0]; Assert.IsNull(ServiceMenuLaunchPlan.Create(action, ["/" + new string('a', DisplaySanitizer.MaxExecutedCharacters)], CultureInfo.InvariantCulture)); var remote = new DesktopApplication("run", "Run", "tool %F", path); - Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(remote, ["smb://host/file.png"]).Count); + Assert.AreEqual(1, DesktopExecExpander.ExpandServiceMenu(remote, ["smb://host/file.png"]).Count); } private static string Fixture(string name) => File.ReadAllText(Path.Combine(AppContext.BaseDirectory, "Mime", "Fixtures", "ServiceMenus", name + ".desktop")); @@ -388,8 +379,7 @@ public async Task Scan_DistroRootMenusKeepAllLiteralActionsAndPinTheirCommands() var folders = await service.GetActionsAsync([fx.Home]); var rootFolders = folders.Where(a => a.Submenu == "Root Actions").ToArray(); Assert.AreEqual(11, rootFolders.Length); - Assert.AreEqual(12, folders.Count); - Assert.AreEqual("RunGhosttyDir", folders.Last().ActionId); + Assert.AreEqual(11, folders.Count); var multipleFolders = await service.GetActionsAsync([fx.Home, fx.Home]); Assert.IsFalse(multipleFolders.Any(a => a.ActionId == "RunGhosttyDir")); Assert.IsTrue(rootFolders.All(a => a.Priority == "TopLevel")); @@ -402,24 +392,9 @@ public async Task Scan_DistroRootMenusKeepAllLiteralActionsAndPinTheirCommands() } [TestMethod] - public void Plan_DistroGhosttyUsesLiteralLongOptionWithoutRewritingExec() + public void Plan_DistroGhosttyRejectsEmbeddedFieldCode() { - using var fx = new XdgFixture(); - var text = Fixture("com.mitchellh.ghostty"); - var path = fx.Write("data/kio/servicemenus/com.mitchellh.ghostty.desktop", text); - var action = ServiceMenuParser.ParseStrict(text.Split('\n'), path, CultureInfo.InvariantCulture)!.Actions.Single(); - var directory = Path.Combine(fx.Home, "a 'quoted' $(touch bad); folder"); - Directory.CreateDirectory(directory); - var plan = ServiceMenuLaunchPlan.Create(action, [directory], CultureInfo.InvariantCulture); - Assert.IsNotNull(plan); - Assert.AreEqual(1, plan.Invocations.Count); - CollectionAssert.AreEqual(new[] { "ghostty", "--working-directory=" + directory, "--gtk-single-instance=false" }, plan.Invocations[0].ToArray()); - Assert.AreEqual("ghostty --working-directory=%F --gtk-single-instance=false", action.Application.Exec); - Assert.IsTrue(plan.Identity.StillMatches(path)); - Assert.AreEqual(1, ServiceMenuParser.ParseStrict(text.Split('\n'), "/other.desktop", CultureInfo.InvariantCulture)!.Actions.Count); - Assert.IsNull(ServiceMenuLaunchPlan.Create(action, [directory, fx.Home], CultureInfo.InvariantCulture)); - var modified = text.Replace("--gtk-single-instance=false", "--gtk-single-instance=false --title=%f"); - Assert.AreEqual(0, ServiceMenuParser.ParseStrict(modified.Split('\n'), path, CultureInfo.InvariantCulture)!.Actions.Count); + Assert.AreEqual(0, Parse(Fixture("com.mitchellh.ghostty"))!.Actions.Count); } [TestMethod] From dff6fcd082a7aee2458633eafd5a63b9022f8c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Tue, 13 Oct 2026 00:59:38 +0300 Subject: [PATCH 4/4] Redirect launched applications' stdio to /dev/null Co-Authored-By: Codex Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../Launching/IProcessStarter.cs | 18 ++++-- .../Launching/DetachedProcessStarterTests.cs | 64 +++++++++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 tests/Files.Platform.Tests/Launching/DetachedProcessStarterTests.cs diff --git a/src/Files.Platform.Linux/Launching/IProcessStarter.cs b/src/Files.Platform.Linux/Launching/IProcessStarter.cs index 63891b2dd430..cb8291703d0f 100644 --- a/src/Files.Platform.Linux/Launching/IProcessStarter.cs +++ b/src/Files.Platform.Linux/Launching/IProcessStarter.cs @@ -30,7 +30,7 @@ public interface IProcessStarter } /// - /// Starts processes through setsid when available, passing arguments directly without a shell. + /// Starts processes through setsid when available, using a fixed shell wrapper to isolate stdio while preserving separate arguments. /// public sealed class DetachedProcessStarter : IProcessStarter { @@ -63,22 +63,26 @@ public Task StartDetachedAsync(ProcessLaunch launch, CancellationToken cancellat { UseShellExecute = false, CreateNoWindow = true, + FileName = "/bin/sh", }; if (launch.WorkingDirectory is not null) startInfo.WorkingDirectory = launch.WorkingDirectory; + // Only this fixed script is parsed by the shell; program and arguments remain positional parameters. + startInfo.ArgumentList.Add("-c"); + startInfo.ArgumentList.Add("exec \"$@\" /dev/null 2>&1"); + startInfo.ArgumentList.Add("linuxfiles-launch"); + if (setsid is not null) { - startInfo.FileName = setsid; + startInfo.ArgumentList.Add(setsid); startInfo.ArgumentList.Add("--fork"); - startInfo.ArgumentList.Add(program); - } - else - { - startInfo.FileName = program; + startInfo.ArgumentList.Add("--"); } + startInfo.ArgumentList.Add(program); + foreach (var argument in launch.Arguments) startInfo.ArgumentList.Add(argument); diff --git a/tests/Files.Platform.Tests/Launching/DetachedProcessStarterTests.cs b/tests/Files.Platform.Tests/Launching/DetachedProcessStarterTests.cs new file mode 100644 index 000000000000..00bb60fdae15 --- /dev/null +++ b/tests/Files.Platform.Tests/Launching/DetachedProcessStarterTests.cs @@ -0,0 +1,64 @@ +// Copyright (c) Files Community +// Licensed under the MIT License. + +using Files.Platform.Linux.Launching; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; +using System.IO; +using System.Threading.Tasks; + +namespace Files.Platform.Tests.Launching +{ + [TestClass] + public sealed class DetachedProcessStarterTests + { + private sealed class TestLocator(bool useSetsid) : IExecutableLocator + { + private readonly PathExecutableLocator inner = new(); + public string? Locate(string command) => command == "setsid" && !useSetsid ? null : inner.Locate(command); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Launch_IsolatesStdioAndPreservesArguments(bool useSetsid) + { + if (!OperatingSystem.IsLinux()) + Assert.Inconclusive("Requires Linux descriptors."); + if (useSetsid && new PathExecutableLocator().Locate("setsid") is null) + Assert.Inconclusive("Requires setsid."); + + var directory = Path.Combine(Path.GetTempPath(), "files-detached-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(directory); + try + { + var script = Path.Combine(directory, "probe ' $().sh"); + var result = Path.Combine(directory, "result"); + // Inspect the launched shell's descriptors before redirecting the report to a file. + await File.WriteAllTextAsync(script, + "#!/bin/sh\n" + + "a=$(readlink /proc/$$/fd/0)\nb=$(readlink /proc/$$/fd/1)\nc=$(readlink /proc/$$/fd/2)\n" + + "printf '%s\\n' \"$a\" \"$b\" \"$c\" \"$PWD\" \"$@\" > result.tmp\n" + + "mv result.tmp result\n"); + File.SetUnixFileMode(script, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + string[] arguments = ["a b", "", "$(touch injected)", "`touch injected`", "'\"; & *", "--option"]; + + await new DetachedProcessStarter(new TestLocator(useSetsid)) + .StartDetachedAsync(new ProcessLaunch(script, arguments, directory)); + + var deadline = DateTime.UtcNow.AddSeconds(10); + while (!File.Exists(result) && DateTime.UtcNow < deadline) + await Task.Delay(20); + Assert.IsTrue(File.Exists(result), "The descriptor probe did not finish."); + CollectionAssert.AreEqual(new string[] { "/dev/null", "/dev/null", "/dev/null", directory, + arguments[0], arguments[1], arguments[2], arguments[3], arguments[4], arguments[5] }, + await File.ReadAllLinesAsync(result)); + Assert.IsFalse(File.Exists(Path.Combine(directory, "injected"))); + } + finally + { + Directory.Delete(directory, recursive: true); + } + } + } +}