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 5bdc356b6839..de7291cd1fcb 100644 --- a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs +++ b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs @@ -14,173 +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"]; - /// - /// 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 (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) { - 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 - { - 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 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 into separate argv entries, never command text. public static IReadOnlyList> Expand(DesktopApplication application, IReadOnlyList targets) { - var exec = application.Exec; - if (string.IsNullOrWhiteSpace(exec)) - 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]; - + // 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].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 = Tokenize(Substitute(application, exec, targets, single, hasShell)); - if (argv is null || argv.Count == 0) - return []; - - result.Add(argv); - } - - 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; } - // 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. @@ -204,87 +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; - } - - 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/src/Files.Platform.Linux/Launching/IProcessStarter.cs b/src/Files.Platform.Linux/Launching/IProcessStarter.cs index dac841ef699a..cb8291703d0f 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, using a fixed shell wrapper to isolate stdio while preserving separate arguments. /// 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,53 +52,44 @@ 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, + 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(sh); - } - else - { - startInfo.FileName = sh; + startInfo.ArgumentList.Add("--"); } - 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 7ba813e29d62..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]); } @@ -87,9 +106,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 +117,27 @@ public void DeprecatedCodes_AreRemoved() CollectionAssert.AreEqual(new[] { "viewer" }, Expand("viewer %d %D %n %N %v %m")[0]); } + [TestMethod] + [DataRow("viewer --file=%f")] + [DataRow("viewer prefix%c")] + [DataRow("viewer \"%f\"")] + [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 SingleCode_IsExpandedOnlyOnce() + { + CollectionAssert.AreEqual(new[] { "viewer", "/x %U $(id)" }, Expand("viewer %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..552b0373e8ab 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,32 @@ public void MultipleFiles_AreSeparateLiteralArguments() } [TestMethod] - public void ShellDashC_ValueIsSingleQuotedInScript() + public void FixedShellScript_TargetsAreLiteralPositionalArguments() { 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]); - } - - [TestMethod] - public void ShellDashC_MultipleFiles_AreEachQuoted() - { - var argv = Expand("bash -c \"app %F\"", "/a b", "/c'd")[0]; - - Assert.AreEqual("app '/a b' '/c'\\''d'", argv[2]); - } + 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); - [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() + [DataRow("sh -c %f")] + [DataRow("env -S %f")] + [DataRow("nice bash --rcfile %f")] + [DataRow("launcher %f")] + public void TrustedCommands_AreNotInspected(string exec) { - var app = new DesktopApplication("t.desktop", "Evil $(x) 'name'", "sh -c \"echo %c %k\"", "/a b/t.desktop"); - - var argv = DesktopExecExpander.Expand(app, [])[0]; - - Assert.AreEqual("echo 'Evil $(x) '\\''name'\\''' '/a b/t.desktop'", argv[2]); + foreach (var name in Nasty) + Assert.AreEqual(Safe(name), Expand(exec, name)[0][^1]); } [TestMethod] 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); + } + } + } +} diff --git a/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs b/tests/Files.Platform.Tests/Launching/LauncherServiceTests.cs index 13b89e74aa4b..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)); @@ -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() { 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]