Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More Windows subprocess mitigations #313

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions System/Process.hs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ module System.Process (
getPid,
getCurrentPid,

-- ** Secure process creation on Windows
-- $windows-mitigations

-- ** Control-C handling on Unix
-- $ctlc-handling

Expand Down Expand Up @@ -379,6 +382,39 @@ processFailedException fun cmd args exit_code =
Nothing Nothing)


-- ----------------------------------------------------------------------------
-- Secure process creation on Windows

-- $windows-migitations
--
-- In general it is strongly advised that any untrusted user input be validated before
-- being passed to a subprocess. One must be especially careful on Windows due to the
-- crude nature of the platform's argument passing scheme. Specifically, unlike POSIX
-- platforms, Windows treats the command-line not as a sequence of arguments but rather
-- as a single string. It is therefore the responsibility of the called process to tokenize
-- this string into distinct arguments.
--
-- While various programs on Windows tend to differ in their precise argument splitting
-- behavior, the scheme used by @process@'s 'RawCommand' 'CmdSpec' should work for
-- most reasonable programs. If you find that 'RawCommand' doesn't provide
-- the behavior you need, it is recommended to instead compose your command-line
-- manually and rather using the 'shell' 'CmdSpec'.
--
-- Additionally, the idiosyncratic escaping and string interpolation behavior of
-- the Windows @cmd.exe@ command interpreter is known to introduce considerable
-- complication to secure process creation. For this reason, @process@ implements
-- specific argument escaping logic when the executable's file extension suggests
-- that it is a batch file (e.g. @.bat@ or @.cmd@). However, this is not a
-- completely reliable mitigation as Windows will also silently execute batch files
-- when starting executables lacking a file extension (e.g. @callProcess "hello" []@
-- when a @hello.bat@ is present in @PATH@). For this reason, users are encouraged to
-- specify the file extension of invoked executables where possible, especially
-- when untrusted input is involved.
--
-- Users passed untrusted input to subprocesses on Windows are encouraged to review
-- <https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/>
-- for guidance on how to safely navigate these waters.

-- ----------------------------------------------------------------------------
-- Control-C handling on Unix

Expand Down
1 change: 1 addition & 0 deletions System/Process/Windows.hsc
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ translateCmdExeArg xs = "^\"" ++ snd (foldr escape (True,"^\"") xs)
where escape '"' (_, str) = (True, '\\' : '"' : str)
escape '\\' (True, str) = (True, '\\' : '\\' : str)
escape '\\' (False, str) = (False, '\\' : str)
escape '%' (_, str) = (False, "%%cd:~,%" ++ str)
escape c (_, str)
| c `elem` "^<>|&()" = (False, '^' : c : str)
| otherwise = (False, c : str)
Expand Down
Loading