Fix BatBadBut vulnerability

- Refuse to execute *.bat/*.cmd
- Refuse to execute cmd.exe without ChildProcessFlags.DisableArgumentQuoting

Fixes #1

src/ChildProcess.ExamplePreview/ChildProcessExamplesWindows.cs

@@ -11,6 +11,18 @@
 
 #pragma warning disable CA1849 // Call async methods when in an async method
 
+// NOTE:
+//
+// When executing "cmd.exe", you need to set DisableArgumentQuoting and escape arguments on your own.
+// You need to escape arguments in a way specific to the command being invoked.
+// There is no standard quoting method possible for "cmd.exe".
+//
+// If arguments originate from an untrusted input, it may be a good idea to avoid executing "cmd.exe".
+// Incorrectly escaped arguments can lead to execution of an arbitrary executable because "cmd.exe /c"
+// takes an arbitrary shell command line.
+//
+// Search "BatBadBut vulnerability" for the background.
+//
 namespace Asmichi
 {
     public static class ChildProcessExamplesWindows
@@ -43,6 +55,7 @@ private static async Task BasicAsync()
                 StdOutputRedirection = OutputRedirection.OutputPipe,
                 // Works like 2>&1
                 StdErrorRedirection = OutputRedirection.OutputPipe,
+                Flags = ChildProcessFlags.DisableArgumentQuoting,
             };
 
             using var p = ChildProcess.Start(si);
@@ -67,7 +80,7 @@ private static async Task RedirectionToFileAsync()
                 StdErrorRedirection = OutputRedirection.File,
                 StdOutputFile = tempFile,
                 StdErrorFile = tempFile,
-                Flags = ChildProcessFlags.UseCustomCodePage,
+                Flags = ChildProcessFlags.UseCustomCodePage | ChildProcessFlags.DisableArgumentQuoting,
                 CodePage = Encoding.Default.CodePage, // UTF-8 on .NET Core
             };
 
@@ -96,7 +109,7 @@ private static async Task TruePipingAsync()
                 StdErrorRedirection = OutputRedirection.Handle,
                 StdOutputHandle = inPipe.ClientSafePipeHandle,
                 StdErrorHandle = inPipe.ClientSafePipeHandle,
-                Flags = ChildProcessFlags.UseCustomCodePage,
+                Flags = ChildProcessFlags.UseCustomCodePage | ChildProcessFlags.DisableArgumentQuoting,
                 CodePage = Encoding.Default.CodePage, // UTF-8 on .NET Core
             };
 

src/ChildProcess.Test/ProcessManagement/ChildProcessTest_Windows.cs

@@ -2,7 +2,9 @@
 
 using System;
 using System.Globalization;
+using System.IO;
 using System.Runtime.InteropServices;
+using System.Text;
 using Asmichi.Interop.Windows;
 using Asmichi.Utilities;
 using Xunit;
@@ -105,5 +107,77 @@ public void RejectsCreateSuspendedOnUnsupportedPlatforms()
 
             Assert.Throws<PlatformNotSupportedException>(() => { ChildProcess.Start(si).Dispose(); });
         }
+
+        [Theory]
+        [InlineData("foo.bat")]
+        [InlineData("foo.BaT")]
+        [InlineData("foo.cmd")]
+        [InlineData("foo.CmD")]
+        public void RejectsBatchFiles(string batchFileName)
+        {
+            // Testing Windows-specific behavior.
+            if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                return;
+            }
+
+            using var temp = new TemporaryDirectory();
+
+            // Create a batch file.
+            var batchFilePath = Path.Join(temp.Location, batchFileName);
+            File.WriteAllText(batchFilePath, "exit /b 1", Encoding.ASCII);
+
+            var si = new ChildProcessStartInfo(batchFilePath)
+            {
+                StdInputRedirection = InputRedirection.NullDevice,
+                StdOutputRedirection = OutputRedirection.NullDevice,
+                StdErrorRedirection = OutputRedirection.NullDevice,
+            };
+
+            Assert.Throws<ChildProcessStartingBlockedException>(() => { ChildProcess.Start(si).Dispose(); });
+        }
+
+        [Theory]
+        [InlineData("cmd.exe")]
+        [InlineData("CmD.ExE")]
+        public void CanStartCmdExeIfDisableArgumentQuoting(string cmdExeFileName)
+        {
+            // Testing Windows-specific behavior.
+            if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                return;
+            }
+
+            var si = new ChildProcessStartInfo(cmdExeFileName, "/c", "echo", "foo")
+            {
+                Flags = ChildProcessFlags.DisableArgumentQuoting,
+                StdInputRedirection = InputRedirection.NullDevice,
+                StdOutputRedirection = OutputRedirection.OutputPipe,
+                StdErrorRedirection = OutputRedirection.NullDevice,
+            };
+
+            Assert.Equal("foo", ExecuteForStandardOutput(si, Encoding.ASCII).Trim());
+        }
+
+        [Theory]
+        [InlineData("cmd.exe")]
+        [InlineData("CmD.ExE")]
+        public void RejectsStartingCmdExeWithoutDisableArgumentQuoting(string cmdExeFileName)
+        {
+            // Testing Windows-specific behavior.
+            if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                return;
+            }
+
+            var si = new ChildProcessStartInfo(cmdExeFileName, "/c", "echo", "foo")
+            {
+                StdInputRedirection = InputRedirection.NullDevice,
+                StdOutputRedirection = OutputRedirection.NullDevice,
+                StdErrorRedirection = OutputRedirection.NullDevice,
+            };
+
+            Assert.Throws<ChildProcessStartingBlockedException>(() => { ChildProcess.Start(si).Dispose(); });
+        }
     }
 }

src/ChildProcess/ProcessManagement/ChildProcess.cs

@@ -21,6 +21,7 @@ public static class ChildProcess
         /// <returns>The started process.</returns>
         /// <exception cref="ArgumentException"><paramref name="startInfo"/> has an invalid value.</exception>
         /// <exception cref="ArgumentNullException"><paramref name="startInfo"/> is null.</exception>
+        /// <exception cref="ChildProcessStartingBlockedException">Starting the child process is blocked. See <see cref="ChildProcessStartingBlockedException"/> for details.</exception>
         /// <exception cref="FileNotFoundException">The executable not found.</exception>
         /// <exception cref="IOException">Failed to open a specified file.</exception>
         /// <exception cref="AsmichiChildProcessLibraryCrashedException">The operation failed due to critical disturbance.</exception>

src/ChildProcess/ProcessManagement/ChildProcessStartInfo.cs

@@ -262,6 +262,15 @@ public ChildProcessStartInfo(string fileName, params string[] arguments)
         /// <para>
         /// (Windows-specific) If <see cref="FileName"/> does not contain an extension, ".exe" is appended.
         /// </para>
+        /// <para>
+        /// (Windows-specific) <see cref="FileName"/> must not be a batch file (".bat"/".cmd").
+        /// See <see cref="ChildProcessStartingBlockedException"/> for details.
+        /// </para>
+        /// <para>
+        /// (Windows-specific) If <see cref="FileName"/> is "cmd.exe" (without the directory part),
+        /// <see cref="ChildProcessFlags.DisableArgumentQuoting"/> must be set.
+        /// See <see cref="ChildProcessStartingBlockedException"/> for details.
+        /// </para>
         /// </summary>
         public string? FileName { get; set; }
 

src/ChildProcess/ProcessManagement/ChildProcessStartingBlockedException.cs

@@ -0,0 +1,67 @@
+// Copyright (c) @asmichi (https://github.com/asmichi). Licensed under the MIT License. See LICENCE in the project root for details.
+
+using System;
+using System.Runtime.Serialization;
+
+namespace Asmichi.ProcessManagement
+{
+    /// <summary>
+    /// Thrown when starting a child process is blocked because it may execute an unexpected program.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// (Windows-specific) Thrown when <see cref="ChildProcessStartInfo.FileName"/> is "cmd.exe" (without the directory part),
+    /// but <see cref="ChildProcessFlags.DisableArgumentQuoting"/> is not set.<br/>
+    /// If arguments originate from an untrusted input, it may be a good idea to avoid executing "cmd.exe".
+    /// Incorrectly escaped arguments can lead to execution of an arbitrary executable because "cmd.exe /c" takes
+    /// an arbitrary shell command line. Search "BatBadBut vulnerability" for the background.<br/>
+    /// If arguments are trusted, set <see cref="ChildProcessFlags.DisableArgumentQuoting"/> and escape arguments on your own.
+    /// Note you need to escape arguments in a way specific to the command being invoked. There is no standard quoting method possible for "cmd.exe".
+    /// </para>
+    /// <para>
+    /// (Windows-specific) Thrown when <see cref="ChildProcessStartInfo.FileName"/> has the ".bat" or ".cmd" extension.
+    /// Supply the batch file to "cmd.exe /c" instead. See also the above paragraph.
+    /// </para>
+    /// </remarks>
+    [Serializable]
+    public class ChildProcessStartingBlockedException : Exception
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ChildProcessStartingBlockedException"/> class.
+        /// </summary>
+        public ChildProcessStartingBlockedException()
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ChildProcessStartingBlockedException"/> class
+        /// with a message.
+        /// </summary>
+        /// <param name="message">Error message.</param>
+        public ChildProcessStartingBlockedException(string message)
+            : base(message)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ChildProcessStartingBlockedException"/> class
+        /// with a message and an inner exception.
+        /// </summary>
+        /// <param name="message">Error message.</param>
+        /// <param name="innerException">Inner exception.</param>
+        public ChildProcessStartingBlockedException(string message, Exception innerException)
+            : base(message, innerException)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ChildProcessStartingBlockedException"/> class with serialized data.
+        /// </summary>
+        /// <param name="info"><see cref="SerializationInfo"/>.</param>
+        /// <param name="context"><see cref="StreamingContext"/>.</param>
+        protected ChildProcessStartingBlockedException(SerializationInfo info, StreamingContext context)
+            : base(info, context)
+        {
+        }
+    }
+}

src/ChildProcess/ProcessManagement/WindowsChildProcessStateHelper.cs

@@ -47,6 +47,17 @@ public unsafe IChildProcessStateHolder SpawnProcess(
 
             Debug.Assert(startInfo.CreateNewConsole || ConsolePal.HasConsoleWindow());
 
+            if (resolvedPath.EndsWith(".bat", StringComparison.OrdinalIgnoreCase)
+                || resolvedPath.EndsWith(".cmd", StringComparison.OrdinalIgnoreCase))
+            {
+                throw new ChildProcessStartingBlockedException("Execution of a batch file ('.bat'/'.cmd') was blocked.");
+            }
+            if (Path.GetFileName(resolvedPath.AsSpan()).Equals("cmd.exe", StringComparison.OrdinalIgnoreCase)
+                && !flags.HasDisableArgumentQuoting())
+            {
+                throw new ChildProcessStartingBlockedException("Execution of 'cmd.exe' without DisableArgumentQuoting was blocked. See the description of ChildProcessStartingBlockedException.");
+            }
+
             var commandLine = WindowsCommandLineUtil.MakeCommandLine(resolvedPath, arguments ?? Array.Empty<string>(), !flags.HasDisableArgumentQuoting());
             var environmentBlock = startInfo.UseCustomEnvironmentVariables ? WindowsEnvironmentBlockUtil.MakeEnvironmentBlock(environmentVariables.Span) : null;