Merge pull request #312 from bgamari/wip/windows-batch-inj

Address Windows command-line injection vulnerability

System/Process.hs

@@ -478,7 +478,8 @@ processFailedException fun cmd args exit_code =
 --
 -- * The command to run, which must be in the $PATH, or an absolute or relative path
 --
--- * A list of separate command line arguments to the program
+-- * A list of separate command line arguments to the program.  See 'RawCommand' for
+--   further discussion of Windows semantics.
 --
 -- * A string to pass on standard input to the forked process.
 --

System/Process/Common.hs

@@ -160,6 +160,14 @@ data CmdSpec
       --   see the
       --   <http://msdn.microsoft.com/en-us/library/windows/desktop/aa365527%28v=vs.85%29.aspx documentation>
       --   for the Windows @SearchPath@ API.
+      --
+      --   Windows does not have a mechanism for passing multiple arguments.
+      --   When using @RawCommand@ on Windows, the command line is serialised
+      --   into a string, with arguments quoted separately.  Command line
+      --   parsing is up individual programs, so the default behaviour may
+      --   not work for some programs.  If you are not getting the desired
+      --   results, construct the command line yourself and use 'ShellCommand'.
+      --
   deriving (Show, Eq)
 
 

System/Process/Windows.hsc

@@ -25,6 +25,7 @@ import Control.Concurrent
 import Control.Exception
 import Control.Monad
 import Data.Bits
+import Data.Char (toLower)
 import Foreign.C
 import Foreign.Marshal
 import Foreign.Ptr
@@ -425,8 +426,11 @@ commandToProcess (ShellCommand string) = do
         -- which partly works.  There seem to be some quoting issues, but
         -- I don't have the energy to find+fix them right now (ToDo). --SDM
         -- (later) Now I don't know what the above comment means.  sigh.
-commandToProcess (RawCommand cmd args) = do
-  return (cmd, translateInternal cmd ++ concatMap ((' ':) . translateInternal) args)
+commandToProcess (RawCommand cmd args)
+  | map toLower (takeExtension cmd) `elem` [".bat", ".cmd"]
+  = return (cmd, translateInternal cmd ++ concatMap ((' ':) . translateCmdExeArg) args)
+  | otherwise
+  = return (cmd, translateInternal cmd ++ concatMap ((' ':) . translateInternal) args)
 
 -- Find CMD.EXE (or COMMAND.COM on Win98).  We use the same algorithm as
 -- system() in the VC++ CRT (Vc7/crt/src/system.c in a VC++ installation).
@@ -467,6 +471,30 @@ findCommandInterpreter = do
                                 "findCommandInterpreter" Nothing Nothing)
       Just cmd -> return cmd
 
+-- | Alternative regime used to escape arguments destined for scripts
+-- interpreted by @cmd.exe@, (e.g. @.bat@ and @.cmd@ files).
+--
+-- This respects the Windows command interpreter's quoting rules:
+--
+-- * the entire argument should be surrounded in quotes
+-- * the backslash symbol is used to escape quotes and backslashes
+-- * the carat symbol is used to escape other special characters with
+--   significance to the interpreter
+--
+-- It is particularly important that we perform this quoting as
+-- unvalidated unquoted command-line arguments can be used to achieve
+-- arbitrary user code execution in when passed to a vulnerable batch
+-- script.
+--
+translateCmdExeArg :: String -> String
+translateCmdExeArg xs = "^\"" ++ snd (foldr escape (True,"^\"") xs)
+  where escape '"'  (_,     str) = (True,  '\\' : '"'  : str)
+        escape '\\' (True,  str) = (True,  '\\' : '\\' : str)
+        escape '\\' (False, str) = (False, '\\' : str)
+        escape c    (_,     str)
+          | c `elem` "^<>|&()"   = (False, '^' : c : str)
+          | otherwise            = (False,       c : str)
+
 translateInternal :: String -> String
 translateInternal xs = '"' : snd (foldr escape (True,"\"") xs)
   where escape '"'  (_,     str) = (True,  '\\' : '"'  : str)

changelog.md

@@ -1,5 +1,13 @@
 # Changelog for [`process` package](http://hackage.haskell.org/package/process)
 
+## 1.6.19.0 *April 2024*
+
+* Adjust command-line escaping logic on Windows to ensure that occurrences of
+  characters with special significance to the Windows batch interpreter are
+  properly escaped in arguments passed to `.bat` and `.cmd` processes.
+  This addresses
+  [HSEC-2024-0003](https://github.com/haskell/security-advisories/tree/main/advisories/hackage/process/HSEC-2024-0003.md).
+
 ## 1.6.18.0 *September 2023*
 
 * Fix deadlock when waiting for process completion and process jobs [#273](https://github.com/haskell/process/issues/273)

process.cabal

@@ -1,5 +1,5 @@
 name:          process
-version:       1.6.18.0
+version:       1.6.19.0
 -- NOTE: Don't forget to update ./changelog.md
 license:       BSD3
 license-file:  LICENSE