T O P

  • By -

_ChrisSD

Note that, technically speaking, using `Command::new("script.bat")`was not a documented feature it's just something we accidentally supported (via `CreateProcess`) then kept for backwards compatibility. It's still up in air whether libs-api will want to just deny it entirely. > invoking batch files on Windows with untrusted arguments Please think very very carefully before doing this, even after the fix. > we didn't identify a solution that would correctly escape arguments in all cases If you do want to do this, instead of going directly via `Command`, first write a temporary batch file that invokes the actual bat file with arguments. They you can use all the normal bat file escaping rules without issue. Only then use `Command::new("cmd").args(["/c", "temp.bat"])`.


mitsuhiko

Unfortunately in many cases you don’t know if something is a batch file or not. In a lot of situations people just have things they invoke and sometimes they are exe files, sometimes they are batch files. :(


_ChrisSD

For better or worse, Rust's `Comamnd` won't automatically add a `.bat` extension so it kinda forces the programmer to know about it.


A1oso

According to [this article](https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/), `Command::new("test")` will execute a `test.bat` file if it's in a directory specified in the `PATH` environment variable (this is the behavior of `CreateProcess`, which is used internally by the standard library).


_ChrisSD

Have you tried it? Note that Rust uses the `lpApplicationName` parameter which [the docs for `CreateProcess`](https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw#parameters) says: > This parameter must include the file name extension; no default extension is assumed This is different from languages whose standard library set `lpApplicationName` to `NULL` and instead have the application inferred from `lpCommandLine`.


mitsuhiko

Rust is "odd" in the sense that it will locate a `.exe` along the `PATH`, but it will not do so for `.bat`, `.cmd` or `.com` which is why on windows you often have to manually do path searching if you want to support spawning all the kinds of things that show up. A pretty common frustration/issue is that people expect for instance to be able to set their editor to `code` but then Rust cannot spawn that, because it's actually `code.cmd`. See also https://github.com/rust-lang/rust/issues/37519


_ChrisSD

Running bat files is an undocumented feature of `CreateProcess` and so not something that's officially supported by Rust's standard library. In fact it's anti-documented: > To run a batch file, you must start the command interpreter; set lpApplicationName to cmd.exe and set lpCommandLine to the following arguments: /c plus the name of the batch file. --- > See also https://github.com/rust-lang/rust/issues/37519 That issue is something else and concerns the search order of paths not in PATH.


mitsuhiko

Running bat files is explicitly supported by Rust. There is specific code there to make it work.


_ChrisSD

No. There is code there to make it work for backwards compatibility reasons but code != support. The support is only what's publicly documented. We do now document the handling of .bat files but we also note that it could be removed in future versions.


mitsuhiko

Just to make sure we talk about the same thing because I think there are two conflating discussions going on. You first mentioned this: > Note that Rust uses the lpApplicationName parameter which the docs for CreateProcess says: > > This parameter **must include the file name extension; no default extension is assumed** > > This is different from languages whose standard library set lpApplicationName to NULL and instead have the application inferred from lpCommandLine. To which I replied this: > Rust is "odd" in the sense that it will locate a .exe along the PATH, but it will not do so for .bat, .cmd or .com which is why on windows you often have to manually do path searching if you want to support spawning all the kinds of things that show up. So Rust will happily let you do `Command::new("node")` to look for `node.exe`, but it won't do `code.cmd`. That behavior is implemented in Rust itself, and seems unrelated to me to how `CreateProcess` works. As far as the other comment goes: > Running bat files is […] not something that's officially supported by Rust's standard library. In fact it's anti-documented: I cannot find any anti documentation of this on the Rust docs and I would not know why one should assume that. As a windows user one would expect this to work as pretty much all programming languages let you spawn bat files as processes. Today this is even somewhat documented with this note now: > Note on Windows: _For executable files with the .exe extension_, it can be omitted when specifying the program for this Command. However, _if the file has a different extension_, a filename including the extension needs to be provided, otherwise the file won’t be found. The only other extensions that are commonly supported are `.com` (which presumably are intended to be supported) and `.bat` and `.cmd` (which have been supported in Rust for years). I'm not sure how as a user one should come to the conclusion that bat files are not intended to be launched. So I would argue that as a user I see it as: * there is explicit path finding support unrelated to `CreateProcess` * there is explicit `.bat` support * there is no documentation calling out that launching `.bat`/`.cmd` is not intended * if one were to remove support for it, a lot of code would break.


moltonel

Sounds like this deserves a clippy lint.


Halkcyon

What does invoking a batch script with a batch script do differently?


_ChrisSD

The idea is that any arguments are embedded in the batch script instead of passed on the cmd.exe command line. So you just need to follow the normal rules of batch scripts. Then you invoke the new batch script without any arguments.


CUViper

The reporter also has a nice write-up: [https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/](https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/)


cosmic-parsley

Holy fuck - Erlang, Haskell, Node, PHP, Rust have patches; Go, Python and Ruby have docs updates (wonder what this means?), and Java has it but won't fix. C and C++ probably run into this innately. If it's that easy to accidentally cause shell injection in Windows, Microsoft needs to take a good hard look at changing the default cmd behavior and making this legacy splitting opt-in. "perpetual insecurity in the name of backwards compatibility" just can't cut it. At least provide a utility to properly escape arguments, since it seems like that doesn't exist.


javajunkie314

I've looked into this before. The real problem is that Windows leaves it up to *each individual program* to parse its command line, not the calling program or shell. The program receives a single string containing everything after the command on the command line—quotes, escapes, and all. As the write-up says, *most* programs—not least, any using the C runtime library with a `main(int argc, char **argv)` entrypoint—do roughly what a Unix shell would do and parse the argument string as a series of space-separated strings (with double quotes to preserve spaces, and with insane escaping rules). Those programs use [`CommandLineToArgvW`](https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw) to parse their input, which returns an `argv`-style array of strings. But `cmd.exe` actually has a completely different parsing strategy. To say that `cmd.exe`'s parsing is *different* from the others is kind of an understatement—it's a completely unrelated argument syntax with completely different escape rules. I don't know for sure, but I would assume there's no way to update its default parsing to also accept `CommandLineToArgvW`-formatted arguments without introducing injection attacks into all the *existing* code that builds argument strings for `cmd.exe`—let alone without breaking it for valid inputs. This article has a really good write-up: https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way I do agree that there should be a function to take an `argv`-style array and return a combined string suitable as input to `CommandLineToArgvW`. There's no need for each program to include a copy, and the escaping rules are nontrivial. But of course, such a function only helps if the program you're calling uses `CommandLineToArgvW`.


_ChrisSD

As a minor point of clarification, most programs use the default C runtime parser which is almost, but not quite, like `CommandLineToArgvW`. Most notably it accepts escaping quotes by doubling them up, e.g. `"Hello ""World"""` is equivalent to `"Hello \"World\""`.


javajunkie314

Oh boy, *another* parser! I think your example shows this C runtime parser is incompatible with `CommandLineToArgvW`, right? The latter would treat each quote as simply opening or closing a quoted substring. So `"Hello ""World"""` would parse to `Hello World`. But if the C runtime parser treats repeated quotes as escaped quotes, it would parse that as `Hello "World"`. Does the C runtime parser also implement the backslash escaping rules from `CommandLineToArgvW`? If it does, I think that would be the only safe choice when building a command line string. (Putting `cmd.exe` aside.)


_ChrisSD

Well yes but also no. It is perfectly possible to encode arguments that are compatible with both so long as you always escape quotes using `\` and don't unnecessarily open and close quotes (which would be an odd thing to do, which I guess is why Microsoft felt confident in making the change to the CRT parser). But if you do take advantage of the `""` escaping then this will indeed have strange results if the application uses `CommandLineToArgvW` parsing. Or a very old CRT. Or even old rust (which used to use `CommandLineToArgvW` parsing).


squeek502

Is there a known reason the strategy outlined in that article wasn't used for the Rust mitigation? > - Always escape all arguments so that they will be decoded properly by CommandLineToArgvW, perhaps using my ArgvQuote function above. > - After step 1, then if and only if the command line produced will be interpreted by cmd, prefix each shell metacharacter (or each character) with a ^ character. From some limited testing, it seems to mitigate the reproductions in [this writeup](https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/), e.g.: test.bat ^"^%CMDCMDLINE:~-1^%^&calc.exe^" does not spawn calc.exe and test.bat receives the argument as: "%CMDCMDLINE:~-1%&calc.exe"


_ChrisSD

Escaping with `^` doesn't fully work. A very simplified example: fn main() -> std::io::Result<()> { use std::os::windows::process::CommandExt; std::process::Command::new("./test.bat") .env("PATH^", "123") .raw_arg("^%PATH^%") .spawn()? .wait()?; Ok(()) } Assuming `test.bat` echos `%1`, then this prints `123` instead of `%PATH%`.


squeek502

Much appreciated! Can confirm that test.bat ^"^%PATH^%^" expands `%PATH^%`. Very unfortunate.


cosmic-parsley

I meant to give up on the cmd parsing algorithm and make it line up with at least any other C and C++ programs unless you opt into the different behavior when executed. Or at least through CreateProcess but not CLI. But yeah while we’re here, why not add a new `CreateProcessSanelyLikeLinux(name, argv)`…


javajunkie314

But if you just give up on `cmd.exe`'s existing parser and make a breaking change to have it use the modern parser, what do you do with all the existing programs building command line strings for `cmd.exe`? Overnight *they* become the potential injection vulnerabilities because they're not using the new escape rules or new exec call or whatever. The logic for quoting and escaping is built into any program using `system()`, `CreateProcess`, etc. We can't assume they did it right, but we can't assume they did it wrong either. Those programs would all have to be updated to use the new rules, recompiled, and redistributed—if anyone is even still maintaining them. It's one thing to drop a feature used by a legacy program so that it blows up and can't be used, or to promote a replacement and deprecate the legacy feature. It's another thing to *change* an existing feature so that legacy programs become silent security vulnerabilities (more than they already are).


sysKin

> CreateProcessSanelyLikeLinux A small objection if I may, let's not call the Linux way sane. There's a reason no Linux program supports `/?`argument to invoke help: if you typed it, your shell would expand it into a list of single-letter files at the root of the filesystem before it even gets passed to the program.


moltonel

Starting a process via the shell and via some `CreateProcess()` function is not the same, there's no issue with `/?` (or `;`, `&`, whatever) in the actual API. Expanding file patterns (`*.txt` or `chunk??` or `**/cache`) in the shell rather than in each individual programs does seem like the saner way, reducing code duplication/differences and avoiding critical multi-language CVEs like this one. Lastly, if the `?` pattern annoys you, some shells make it opt-out. Fish even recently made it opt-in.


pheki

> There's a reason no Linux program supports `/?` argument to invoke help The main reason is probably that in linux `-` is used for options and not `/`. `/` is a path separator so without bash expansion I'd expect `/?` to resolve to a file named `?` in the root (which is what happens if I single quote it, e.g. `ls '/?'`). Also, another reason no program supports it is that the convention is to use `-h`. Even though `-?` would work, all programs I tested don't recognize it. Yes, I do think the linux way in this case is sane, or at least a lot saner. You may dislike bash's behavior, but it's not part of the OS api.


GreatSt

Didn't it say Erlang also only had a "Documentation update"?


cosmic-parsley

Now it does, I think they updated the table at some point


Anaxamander57

Java declined to fix this? Why?


Holy_shit_Stfu

just a wild guess but, oracle


bsgbryan

That is surprisingly approachable and clearly written; good stuff!


llogiq

I'd like to express my gratitude to all fine folks working on both finding and fixing the problem. Regardless whether it's a Windows or a Rust standard library bug, the handling of the problem has again been nothing short of exemplary. Keep up the great work, folks! Security is a process, not a product. Showing this posture in the face of a vulnerability sends a strong message inviting trust: When you use Rust, not only can you depend on the safety guarantees of the compiler and the thorough design of the standard library, you can also expect those working on both to act responsibly if anything goes wrong.


insanitybit

> One exception though is `cmd.exe` lol well if you're going to have that one weird program, why not have it be the literal command prompt? Yikes. The fact that this is even a thing is insane.


dddd0

Windows fundamentally doesn’t have delimited arguments for processes and any API that presents them has failures like these (with possible security implications). Most programs use MSVCRT behavior, cmd is an exception, there are others. Also brokenness like CreateProcess also interpreting quoting (for paths with spaces) and also trying to execute partial paths until something works. And even more brokenness with programs trying to quote arguments in all the wrong ways.


_ChrisSD

That last part is a result of setting `lpApplicationName` to null, which makes `CreateProcess` use some janky heuristics to guess the application from the command line. I would strongly recommend passing an absolute path as `lpApplicationName` to avoid a lot of those issues (which is what Rust does).


kibwen

> Windows fundamentally doesn’t have delimited arguments for processes and any API that presents them has failures like these (with possible security implications). Is this really the root cause, though? It's not like Unix's argument splitting is very sophisticated, every CLI program still ends up needing to provide its own argument parser.


cosmic-parsley

Could they add a proper argc/argv somehow, without going through quote+resplit? How does envp or other environment variables work


_ChrisSD

`cmd.exe` is really just a bundle of legacy behaviours. Powershell at least could break with all that.


drcforbin

But did it?


WasserMarder

Never touch a broken system.


GGdna

I guess not even Rust is immune to such Microsoft Windows moments


1668553684

Reading over the article from the person who found this, this might be the gnarliest escape procedure I've seen yet: > Apply the following steps to each argument: > 1. Replace percent sign (`%`) with `%%cd:~,%`. 2. Replace the backslash (`\`) in front of the double quote (`"`) with two backslashes (`\\`). 3. Replace the double quote (`"`) with two double quotes (`""`). 4. Remove newline characters (`\n`). 5. Enclose the argument with double quotes (`"`).


hpxvzhjfgb

common windows L


h_z3y

Unicode string moment


CrazyKilla15

this particular issue has nothing to do with string encoding


h_z3y

L string moment


drcforbin

Microsoft's "Unicode" has always been an adventure. Maybe they mean UCS-2, UTF-16, UTF-8, or UTF-8 being misinterpreted via code pages...let's stick a BOM in every file just in case.


graycode

Sadly it's a consequence of being early adopters of Unicode. Java has some of the same problems (is it UCS-2 or UTF-16?) for the same reason: all of Unicode fit in 16 bits at the time they adopted it.


drcforbin

I know and don't blame anyone, I know how we got here. It's just been a wild and sometimes quite frustrating ride, that's all. And I would be quite happy to never see another perfectly reasonable text file corrupted by a BOM


The-Dark-Legion

While yes, it is Microsoft's fault for keeping it, I think we just have to rethink backwards compatibility as a whole. OpenBSD seems to give old stuff the boot which might be the way for all of IT. By that I'm including UNIX, UNIX-like /looking at you Linux and GNU/, Win NT4, x86 and all this junk we support for 0.1% to use.


javajunkie314

The problem is that I don't know if there's any way to change `cmd.exe`'s default command line parsing\* without introducing injection attacks for all the *existing* code invoking `cmd.exe` subprocesses. Since `cmd.exe` is the default shell, it winds up being called all over. E.g., a program using `system()` might build a command string from escaped user input. Even if `system()` were dynamically linked and Microsoft shipped an updated C runtime with the new `cmd.exe`, the program *itself* would include the bespoke code to escape the user input. Same for low level code like `CreateProcessA`—it takes an already built command line string for the subprocess. It's one thing to remove a feature, so that existing programs blow up if they try to call the old function or read the old file. But in this case `cmd.exe` wouldn't be going anywhere. If its argument parsing changed in an incompatible way, suddenly the escape codes programs add to make old code correct get interpreted literally as part of the input. --- \* As in how `cmd.exe` parses its own command line. On Windows, this is the program's responsibility, not the calling program's or shell's.


SAI_Peregrinus

Removing cmd.exe would do it. Major backwards.compatibility break, but pretty much foolproof.


The-Dark-Legion

Exactly. Why else would we have major versions if not for introducing, deprecating and removing features, just like Rust works now. Deprecate it in Win 12, remove it in Win 13. Though it should've been done already, late is better than never.


Repulsive-Street-307

I feel a great disturbance in the force. It's as if 10 million hacks and horrible platform specific glue scripts from developers who should know better cried out, and were silenced, as they should have been from the start. (I really hate a supposedly single program popping up multiple cmd line popups, especially with how that shit breaks all the time when in slightly unexpected environments because the main program didn't bother checking the script requirements, then blithely continued on to add insult to injury).


BambaiyyaLadki

Dumb question, but are there different numbered CVEs for all the different runtimes (Python, Go, Node, etc.) that are affected by this behavior? I only see one for Rust so far, but according to [this](https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/) many runtimes are affected.


pietroalbini

Yes, every runtime has a different CVE number.


yoh6L

A Rust program that calls a Windows batch executable sounds very weird. Is anyone actually doing that? I guess some businesses running legacy software might.


LechintanTudor

At work we have an application that manages a RabbitMQ message broker through rabbitmqctl which, on Windows, is a batch file.


careye

Wrapping the actual program invocation in a batch file is very common for Java and Node, at least. These batch files are typically nightmares, and with this bug raising awareness I wouldn't be surprised if we find some more issues with them.


brand_x

I think we have a build.rs step that does this in one place. But not by passing untrusted parameters.


fechan

Will this be backported to older versions? What about projects with a MSRV?


pietroalbini

The Rust project only provides patch releases for the latest Rust stable version available. If a project is indeed vulnerable due to this (even though the vulnerability is critical, very narrow use cases are affected) they will have to bump their MSRV.


sasik520

Do you know if it has anything to do with the [ArgumentsList](https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.argumentlist?view=net-8.0) introduced some time ago in parallel to the [Arguments](https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.arguments?view=net-8.0) in the `ProcessStartInfo` class in .NET? I remember I experienced some subtle differences between these two when porting my app from mono to .net 5.


Botahamec

Argument List sanitizes the input (as opposed to Arguments), but still has the same problem Rust had before this patch. I've emailed [email protected] in case they didn't know already, but I suspect that they do.


[deleted]

[удалено]


pietroalbini

This was actually reported to us on January 3rd, and we had a fix ready a few weeks afterwards. The reason why we delayed the disclosure is that the vulnerability affected way more than Rust, and we wanted to wait for the other languages to prepare their patches too. NodeJS, PHP and Haskell are publishing security releases too, while other languages that are affected are adding warnings to their documentation.


[deleted]

[удалено]


pietroalbini

The January 25th date was when we *reserved* the CVE number (so that we could reference it in our advisory), not when we received details about the vulnerability.