From 3c419f9eeedac024c9dccce544e5a6fb587179a5 Mon Sep 17 00:00:00 2001 From: Ben Gamari Date: Mon, 8 Apr 2024 17:02:29 -0400 Subject: [PATCH 1/3] Address command-line injection vulnerability The `process` library on Windows is vulnerable to a command injection vulnerability, via `cmd.exe`'s interpretation of arguments. Processes that invoke batch files (`.bat`, `.cmd`) and pass arguments whose values are affected by program inputs may be affected. Add some additional escaping to neutralise this scenario. Also add some additional library documentation explaining how arguments are processed on Windows. Co-authored-By: Fraser Tweedale HSEC-identifier: HSEC-2024-0003 --- System/Process.hs | 3 ++- System/Process/Common.hs | 8 ++++++++ System/Process/Windows.hsc | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/System/Process.hs b/System/Process.hs index 18892893..79bfa74d 100644 --- a/System/Process.hs +++ b/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. -- diff --git a/System/Process/Common.hs b/System/Process/Common.hs index 61f2f1b7..e2490d85 100644 --- a/System/Process/Common.hs +++ b/System/Process/Common.hs @@ -160,6 +160,14 @@ data CmdSpec -- see the -- -- 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) diff --git a/System/Process/Windows.hsc b/System/Process/Windows.hsc index f6c97b2e..86d3eb6c 100644 --- a/System/Process/Windows.hsc +++ b/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) From 7e616c5c3b4092428d37d3388b792f718d56b10b Mon Sep 17 00:00:00 2001 From: Ben Gamari Date: Mon, 8 Apr 2024 19:32:16 -0400 Subject: [PATCH 2/3] Add changelog entry for Windows escaping changes --- changelog.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/changelog.md b/changelog.md index 38991d75..48908c54 100644 --- a/changelog.md +++ b/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) From eee2752725e1244c2c77467a5e5998df6701349c Mon Sep 17 00:00:00 2001 From: Ben Gamari Date: Mon, 8 Apr 2024 19:33:14 -0400 Subject: [PATCH 3/3] Bump version to 1.6.19.0 --- process.cabal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/process.cabal b/process.cabal index d59fadbc..0b25762f 100644 --- a/process.cabal +++ b/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