29037 Commits

Author SHA1 Message Date
Ryan Liptak
422464d540 std.process.Child: Mitigate arbitrary command execution vulnerability on Windows (BatBadBut)
> Note: This first part is mostly a rephrasing of https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/
> See that article for more details

On Windows, it is possible to execute `.bat`/`.cmd` scripts via CreateProcessW. When this happens, `CreateProcessW` will (under-the-hood) spawn a `cmd.exe` process with the path to the script and the args like so:

    cmd.exe /c script.bat arg1 arg2

This is a problem because:

- `cmd.exe` has its own, separate, parsing/escaping rules for arguments
- Environment variables in arguments will be expanded before the `cmd.exe` parsing rules are applied

Together, this means that (1) maliciously constructed arguments can lead to arbitrary command execution via the APIs in `std.process.Child` and (2) escaping according to the rules of `cmd.exe` is not enough on its own.

A basic example argv field that reproduces the vulnerability (this will erroneously spawn `calc.exe`):

    .argv = &.{ "test.bat", "\"&calc.exe" },

And one that takes advantage of environment variable expansion to still spawn calc.exe even if the args are properly escaped for `cmd.exe`:

    .argv = &.{ "test.bat", "%CMDCMDLINE:~-1%&calc.exe" },

(note: if these spawned e.g. `test.exe` instead of `test.bat`, they wouldn't be vulnerable; it's only `.bat`/`.cmd` scripts that are vulnerable since they go through `cmd.exe`)

Zig allows passing `.bat`/`.cmd` scripts as `argv[0]` via `std.process.Child`, so the Zig API is affected by this vulnerability. Note also that Zig will search `PATH` for `.bat`/`.cmd` scripts, so spawning something like `foo` may end up executing `foo.bat` somewhere in the PATH (the PATH searching of Zig matches the behavior of cmd.exe).

> Side note to keep in mind: On Windows, the extension is significant in terms of how Windows will try to execute the command. If the extension is not `.bat`/`.cmd`, we know that it will not attempt to be executed as a `.bat`/`.cmd` script (and vice versa). This means that we can just look at the extension to know if we are trying to execute a `.bat`/`.cmd` script.

---

This general class of problem has been documented before in 2011 here:

https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way

and the course of action it suggests for escaping when executing .bat/.cmd files is:

- Escape first using the non-cmd.exe rules
- Then escape all cmd.exe 'metacharacters' (`(`, `)`, `%`, `!`, `^`, `"`, `<`, `>`, `&`, and `|`) with `^`

However, escaping with ^ on its own is insufficient because it does not stop cmd.exe from expanding environment variables. For example:

```
args.bat %PATH%
```

escaped with ^ (and wrapped in quotes that are also escaped), it *will* stop cmd.exe from expanding `%PATH%`:

```
> args.bat ^"^%PATH^%^"
"%PATH%"
```

but it will still try to expand `%PATH^%`:

```
set PATH^^=123
> args.bat ^"^%PATH^%^"
"123"
```

The goal is to stop *all* environment variable expansion, so this won't work.

Another problem with the ^ approach is that it does not seem to allow all possible command lines to round trip through cmd.exe (as far as I can tell at least).

One known example:

```
args.bat ^"\^"key^=value\^"^"
```

where args.bat is:

```
@echo %1 %2 %3 %4 %5 %6 %7 %8 %9
```

will print

```
"\"key value\""
```

(it will turn the `=` into a space for an unknown reason; other minor variations do roundtrip, e.g. `\^"key^=value\^"`, `^"key^=value^"`, so it's unclear what's going on)

It may actually be possible to escape with ^ such that every possible command line round trips correctly, but it's probably not worth the effort to figure it out, since the suggested mitigation for BatBadBut has better roundtripping and leads to less garbled command lines overall.

---

Ultimately, the mitigation used here is the same as the one suggested in:

https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/

The mitigation steps are reproduced here, noted with one deviation that Zig makes (following Rust's lead):

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).~~
  - Instead, `\n`, `\r`, and NUL are disallowed and will trigger `error.InvalidBatchScriptArg` if they are found in `argv`. These three characters do not roundtrip through a `.bat` file and therefore are of dubious/no use. It's unclear to me if `\n` in particular is relevant to the BatBadBut vulnerability (I wasn't able to find a reproduction with \n and the post doesn't mention anything about it except in the suggested mitigation steps); it just seems to act as a 'end of arguments' marker and therefore anything after the `\n` is lost (and same with NUL). `\r` seems to be stripped from the command line arguments when passed through a `.bat`/`.cmd`, so that is also disallowed to ensure that `argv` can always fully roundtrip through `.bat`/`.cmd`.
5. Enclose the argument with double quotes (").

The escaped command line is then run as something like:

    cmd.exe /d /e:ON /v:OFF /c "foo.bat arg1 arg2"

Note: Previously, we would pass `foo.bat arg1 arg2` as the command line and the path to `foo.bat` as the app name and let CreateProcessW handle the `cmd.exe` spawning for us, but because we need to pass `/e:ON` and `/v:OFF` to cmd.exe to ensure the mitigation is effective, that is no longer tenable. Instead, we now get the full path to `cmd.exe` and use that as the app name when executing `.bat`/`.cmd` files.

---

A standalone test has also been added that tests two things:

1. Known reproductions of the vulnerability are tested to ensure that they do not reproduce the vulnerability
2. Randomly generated command line arguments roundtrip when passed to a `.bat` file and then are passed from the `.bat` file to a `.exe`. This fuzz test is as thorough as possible--it tests that things like arbitrary Unicode codepoints and unpaired surrogates roundtrip successfully.

Note: In order for the `CreateProcessW` -> `.bat` -> `.exe` roundtripping to succeed, the .exe must split the arguments using the post-2008 C runtime argv splitting implementation, see https://github.com/ziglang/zig/pull/19655 for details on when that change was made in Zig.
2024-04-23 03:21:51 -07:00
Ryan Liptak
84f4c5d9cc std.unicode: Fix ArrayList functions when using populated ArrayLists
ensureTotalCapacityPrecise only satisfies the assumptions made in the ArrayListImpl functions (that there's already enough capacity for the entire converted string if it's all ASCII) when the ArrayList has no items, otherwise it would hit illegal behavior.
2024-04-23 03:20:38 -07:00
Andrew Kelley
a2b834e8c7
Merge pull request #19654 from jacobly0/wasi-ver
Target: add wasi os version
2024-04-15 16:14:22 -07:00
Andrew Kelley
b78b2689ed
Merge pull request #19655 from squeek502/windows-argv-post-2008
ArgIteratorWindows: Match post-2008 C runtime rather than `CommandLineToArgvW`
2024-04-15 15:28:33 -07:00
Alex Kladov
ff18103ef6 std: improve std.once tests
* fix UB when Thread.spawn fails, and we try to join uninitialized
  threads (through new `thread_count` variable).
* make sure that the tests pins down synchronization guarantees: in the
  main thread, we can observe `1` due to synchronization from
  `Thread.join()`. To make sure that once uses Acq/Rel, and not just
  relaxed, we should also additionally check that each thread observes
  1, regardless of whether it was the one to call once.
2024-04-15 15:24:30 -07:00
Jakub Konka
1e5075f812 lib/std/Build/Step/CheckObject: dump section as string 2024-04-15 18:06:44 +02:00
Linus Groh
d483ba7250 zig init: Replace deprecated LazyPath.path with Build.path() 2024-04-15 16:55:04 +03:00
jd
d23a8e83d4 std.Build.Step.ConfigHeader: better error message 2024-04-15 04:10:13 -07:00
Ryan Liptak
cffe1999c6 windows_argv standalone test: Only test against MSVC if it's available 2024-04-15 02:09:48 -07:00
Ryan Liptak
f4c4c04f1c ArgIteratorWindows: Match post-2008 C runtime rather than CommandLineToArgvW
On Windows, the command line arguments of a program are a single WTF-16 encoded string and it's up to the program to split it into an array of strings. In C/C++, the entry point of the C runtime takes care of splitting the command line and passing argc/argv to the main function.

https://github.com/ziglang/zig/pull/18309 updated ArgIteratorWindows to match the behavior of CommandLineToArgvW, but it turns out that CommandLineToArgvW's behavior does not match the behavior of the C runtime post-2008. In 2008, the C runtime argv splitting changed how it handles consecutive double quotes within a quoted argument (it's now considered an escaped quote, e.g. `"foo""bar"` post-2008 would get parsed into `foo"bar`), and the rules around argv[0] were also changed.

This commit makes ArgIteratorWindows match the behavior of the post-2008 C runtime, and adds a standalone test that verifies the behavior matches both the MSVC and MinGW argv splitting exactly in all cases (it checks that randomly generated command line strings get split the same way).

The motivation here is roughly the same as when the same change was made in Rust (https://github.com/rust-lang/rust/pull/87580), that is (paraphrased):

- Consistent behavior between Zig and modern C/C++ programs
- Allows users to escape double quotes in a way that can be more straightforward

Additionally, the suggested mitigation for BatBadBut (https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/) relies on the post-2008 argv splitting behavior for roundtripping of the arguments given to `cmd.exe`. Note: it's not necessary for the suggested mitigation to work, but it is necessary for the suggested escaping to be parsed back into the intended argv by ArgIteratorWindows after being run through a `.bat` file.
2024-04-15 02:09:48 -07:00
Andrew Kelley
64b9893582
Merge pull request #19657 from ziglang/fs.Dir.Walker-null
std.fs.Dir.Walker: maintain a null byte in path names
2024-04-15 00:25:23 -07:00
Andrew Kelley
36060a774e fix namespacing of std.fs.Dir.Walker.Entry 2024-04-14 18:08:42 -07:00
Andrew Kelley
10106660e9 std.fs.Dir.Walker: maintain a null byte in path names 2024-04-14 17:59:43 -07:00
Jacob Young
60d5001ac8 wasi: change default os version to 0.1.0
This version represents "WASI Preview 1".

Closes #19581
2024-04-14 18:43:52 -04:00
Jacob Young
533f54c68e Target: cleanup 2024-04-14 15:33:46 -04:00
Frank Denis
e45bdc6bd6
std.crypto.pcurves.*: simpler, smaller, faster u64 addition with carry (#19644)
signature/s:

Algorithm        Before     After
---------------+---------+-------
ecdsa-p256        3707       4396
ecdsa-p384        1067       1332
ecdsa-secp256k1   4490       5147

Add ECDSA to the benchmark by the way.
2024-04-14 01:13:22 +02:00
Andrew Kelley
a59ad719d2 std.Build.Step.ConfigHeader: better error message 2024-04-13 05:14:08 -07:00
Andrew Kelley
54d1a529f6
Merge pull request #19637 from ziglang/http-host-port
std.http.Client: omit port in http host header sometimes
2024-04-13 03:39:35 -07:00
Андрей Краевский
7cc0e6d4cd
std: fix big int llshr to respect aliasing (#19612) 2024-04-13 03:06:23 -07:00
Ian Johnson
4fac5bd601 Autodoc: fix root module name in sources.tar
This was overlooked in #19458. Using the fully qualified name of each
module usually makes sense, but there is one module where it does not,
namely, the root module, since its name is `root`. The original Autodoc
tar creation logic used `comp.root_name` for the root module back when
it was the only module included in `sources.tar`, and that made sense.
Now, we get the best of both worlds, using the proper root name for the
root module while using the module name for the rest.
2024-04-12 23:43:57 -07:00
Andrew Kelley
0a3dff8125 Revert "std.http.Client: always omit port when it matches default"
This reverts commit db0a42b558c64eac2b4e41d02b078931b0c63af8, but keeps
the changes to std/Uri.zig.
2024-04-12 22:37:07 -07:00
Andrew Kelley
6deb3e3986 std.http.Client: always omit port when it matches default
This makes the host http header have the port if and only if it differs
from the defaults based on the protocol.

This is an alternate implementation that closes #19624.
2024-04-12 22:37:07 -07:00
Andrew Kelley
419753f45e std.http.Client: pass port to server based on user input
This makes the host http header have the port if and only if the URI
provided by the API user included it.

Closes #19624
2024-04-12 22:37:07 -07:00
Jacob Young
f1c0f42cdd cbe: fix optional codegen
Also reduce ctype pool string memory usage, remove self assignments, and
enable more warnings.
2024-04-13 01:35:20 -04:00
travisstaloch
05d9755766
translate-c: allow str literals in bool expressions
this is a follow up to #19610 with fix suggested by Vexu in
https://github.com/ziglang/zig/issues/14642#issuecomment-2048999384
2024-04-12 10:10:42 +00:00
Andrew Kelley
10ff81c264
Merge pull request #19623 from ziglang/LazyPath
std.Build.LazyPath: upgrade API usages of source-relative path
2024-04-11 23:35:51 -07:00
Andrew Kelley
3bafc4400a std.debug.panic: pass the args
Why was this passing null? These values are available and useful.
2024-04-11 23:33:38 -07:00
Igor Anić
9cfac4718d fetch: combine unpack error validation in a single function
Follow up of: #19500
[discussion](https://github.com/ziglang/zig/pull/19500#discussion_r1558238138)
2024-04-11 15:44:40 -07:00
Jeremia Dominguez
0ac15b9726 Document lazy dependency in init build.zig.zon
Also added the same description to doc/build.zig.zon.md
2024-04-11 14:56:07 -07:00
Nguyễn Gia Phong
5b9579845d Document packed struct's backing int declaration 2024-04-11 14:45:28 -07:00
Brandon Black
271d896446 std.hash.crc: get rid of usingnamespace
This flips things around such that std/hash/crc.zig is generated
by the catalog-based generation tool, and the real code that used
to be in that file is moved out to std/hash/crc/impl.zig.  The
generated tests are moved to std/hash/crc/test.zig.  By going this
route, we eliminate the need for usingnamespace without changing
anything for callers of these interfaces.  The Crc32 tests are
simply added to the fixed part of the generated output and
compactified a bit.

This was the second-to-last usage of usingnamespace left in std.
2024-04-11 14:42:04 -07:00
leap123
786876c05e Fix stack iterator on UEFI
Don't know why UEFI wasn't excluded but freestanding is, probably an oversight since I want to have detailed debug info on my panic function on my Headstart bootloader.
2024-04-11 14:21:15 -07:00
Andrew Kelley
499a202a13 std.Build: revert the breaking changes only
The previous commit deleted the deprecated API and then made all the
follow-up changes; this commit reverts only the breaking API changes.
This commit can be reverted once 0.12.0 is tagged.
2024-04-11 14:09:58 -07:00
Andrew Kelley
b30ad74908 remove deprecated LazyPath.path union tag 2024-04-11 14:02:47 -07:00
travisstaloch
3d1652070a
translate-c: support macro with 'assert(false && "error message")'
closes #14642 with modified fix suggested by Vexu in
https://github.com/ziglang/zig/issues/14642#issuecomment-1775476042
2024-04-11 14:39:47 +00:00
andrewkraevskii
f7a76bdfe3 autodoc: fix tokenizer 2024-04-10 20:44:10 -07:00
Andrew Kelley
7fb5a0b18b introduce std.Build.path; deprecate LazyPath.relative
This adds the *std.Build owner to LazyPath so that lazy paths returned
from a dependency can be used in the application without friction or
footguns.

closes #19313
2024-04-10 15:02:20 -07:00
Jacob Young
c4587dc9f4 Uri: propagate per-component encoding
This allows `std.Uri.resolve_inplace` to properly preserve the fact
that `new` is already escaped but `base` may not be.  I originally tried
just moving `raw_uri` around, but it made uri resolution unmanagably
complicated, so I instead added per-component information to `Uri` which
allows extra allocations to be avoided when constructing uris with
components from different sources, and in some cases, deferring the work
all the way to when the uri is printed, where an allocator may not even
be needed.

Closes #19587
2024-04-10 02:11:54 -07:00
Andrew Kelley
215de3ee67
Merge pull request #19500 from ianic/package_filter_errors
package manager: filter unpack errors on paths excluded by manifest
2024-04-09 13:22:48 -07:00
Andrew Kelley
fc17402919
std.crypto.Certificate: support 3072 bits RSA certificate (#19591)
Used by musicbrainz.org API.
2024-04-09 12:16:45 -07:00
Frank Denis
9d27f34d04
crypto.sha3: implement constructions from NIST SP 800-185 (#19533)
https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-185.pdf

This adds useful standard SHA3-based constructions from the
NIST SP 800-185 document:

- cSHAKE: similar to the SHAKE extensible hash function, but
with the addition of a context parameter.
- KMAC: SHAKE-based authentication / keyed XOF
- TupleHash: unambiguous hashing of tuples

These are required by recent protocols and specifications.

They also offer properties that none of the currently available
constructions in the stdlib offer, especially the ability to safely
hash tuples.

Other keyed hash functions/XOFs will fall back to using HMAC, which
is suboptimal from a performance perspective, but fine from a
security perspective.
2024-04-09 12:16:19 -07:00
Igor Anić
4151e6c31b fetch: use arena allocator for diagnostic/UnpackResult
Reference:
https://github.com/ziglang/zig/pull/19500#discussion_r1556476973

Arena is now used for Diagnostic (tar and git). `deinit` is not called on Diagnostic
allowing us to reference strings from Diagnostic in UnpackResult without
dupe.

That seamed reasonable to me. Instead of using gpa for Diagnostic, and
then dupe to arena. Or using arena for both and making dupe so we can deinit
Diagnostic.
2024-04-09 15:00:22 +02:00
Igor Anić
f8dd2a1064 fetch: add executable bit test 2024-04-09 15:00:22 +02:00
Igor Anić
8c58b8fe01 fetch: refactor package root in errors
Use stripRoot in less places. Strip it while copying error from
diagnostic to unpack result so other palaces can be free of this logic.
2024-04-09 15:00:22 +02:00
Igor Anić
b422e4a202 fetch: save test cases
Prepare test cases, store them in Fetch/testdata.
They cover changes in this PR as well from previous one #19111.
2024-04-09 15:00:22 +02:00
Igor Anić
3d5a9237f7 fetch: use empty string instead of null for root_dir
Make it consistent with Cache.Path sub_path.
Remove null check in many locations.
2024-04-09 15:00:22 +02:00
Igor Anić
5f0b434f90 fetch: remove root_dir from error messages
To be consistent with paths in manifest.
2024-04-09 15:00:22 +02:00
Igor Anić
22e9c50376 fetch: fix test tarball
Should include folder structure, at least root folder so it can be found
in pipeToFileSystem.
2024-04-09 15:00:21 +02:00
Igor Anić
fc745fb05c fetch: remove test with mount and net dependencies 2024-04-09 15:00:21 +02:00
Igor Anić
84fac2242c fix zig fmt 2024-04-09 15:00:21 +02:00