A backdoor in xz [LWN.net]

A backdoor in xz

Posted Mar 30, 2024 14:35 UTC (Sat) by smurf (subscriber, #17840)
In reply to: A backdoor in xz by himi
Parent article: A backdoor in xz

> I'm not sure how practical it would be to completely avoid having binary blobs of test data in the repository

Nobody requested that. The problem here is a binary blob that (a) is executed as part of the test (b) isn't even checked in (c) is able to modify the built artefacts.

All three of those are red flags. Preventing any one of them would have disabled this attack. In fact, checking out the git repo (with its immutable commit ID and all that) instead of downloading a tarball which is only loosely (too loosely as it turns out) associated with the release in question could, and should really, have prevented this from affecting Debian (and probably others).

to post comments

A backdoor in xz

Posted Mar 30, 2024 20:10 UTC (Sat) by mjg59 (subscriber, #23239) [Link] (1 responses)

This is inaccurate. The backdoor code is checked in, and isn't able to modify the artifacts in itself. What isn't checked in is a backdoored m4 macro that injects code into configure, which in turn extracts further code from the test files and injects those into the build. It's kind of expected that configure is able to influence the build process, and compressor test files are inherently going to be binaries, so really the only red flag was the difference between the git repo and the tarball - a difference that's largely expected, given how autotools works.

A backdoor in xz

Posted Mar 30, 2024 23:56 UTC (Sat) by chrisoofar (guest, #170494) [Link]

There is a lot of valid conversation around the viability of the autoconf build stack that should not be discounted, and it's time to put it to bed just like programmers left JCL job cards.

And if we're going to insist on a build tool with its own scripting language, then for the love of God and all that is holy pick or construct something that enforces readability with an iron fist.

A backdoor in xz

Posted Mar 31, 2024 1:00 UTC (Sun) by himi (subscriber, #340) [Link] (6 responses)

We want to avoid binary blobs in the source repository because we want the entire repository to be human readable, and therefore at least notionally subject to human review - it's not ridiculous to consider the practicality of the idea. Particularly for cases like this where it's a binary file format which obviously needs binary data as input for at least some of its test cases. It's definitely /possible/, and it may be desirable in the abstract, but how practical it'd be, how much value it'd add, and whether it'd be worth the disruption are the questions that need answering. Certainly it would mitigate against this particular attack, and raises the bar for any attack that wanted to hide a payload in plain sight, but it'd need careful consideration before being recommended as a matter of course.

There are two reasonably simple mitigations that /could/ be done (and probably should be done) to mitigate against this kind of attack: building from reproducible and traceable source (i.e. a git checkout or a git archive with enough metadata that the contents can be verified against a trusted git repo); and doing the actual build in a sanitised environment with no access to anything not required for the build. The obvious people to implement these mitigations are the distros, but they should probably be standard practise for anyone who's creating binary distributables.

'git archive' already does most of what we'd want, but it'd need to add metadata to make the archive contents more readily verifiable, and probably a bunch of smaller tweaks to make it reliable and reproducible for this use case; it probably also needs a properly standardised output format, to avoid issues like we saw with github's changing "release archives". Git would also need to complete the move away from vulnerable digest algorithms, though in practise the "vulnerable" digests are probably still a sufficiently high barrier against almost all attackers. Using a repo checkout directly /isn't/ a good idea - unless you set it up carefully you'd be exposing everything in the repo to your build; this would also make any hash collision attack against the digest algorithm far more valuable to an attacker. An archive mostly avoids those issues, as well as being more easily redistributable.

A sanitised package build environment is something that would mostly be implemented by the distros rather than the developers. It /should/ be pretty simple to implement, even if you want to be able to run the test suite using the newly built binaries: make an initial copy of the source tree, sanitise it, do the build, then retrieve the build artefacts; make another clean copy of the source tree, drop the build artefacts back in, then run the test suite; assuming it passes, package up the build artefacts and you're done. Again, it mitigates against a replay of this attack, and raises the bar a /lot/ for future attacks that want to use the build system to inject an obfuscated payload that's been added to the source distribution, but it does so in a way that can be applied to just about all package builds without needing to change the entire world.

The biggest challenge is how to sanitise the build environment - the best approach would have the developers change the organisation of the source tree to make it easier to sanitise; without that, it'd be a long hard slog to examine each and every package to determine the bare minimum that was required for a reproducible build. I imagine a lot of the grunt work could be automated, but the results would still need to be validated by a human, and re-validated each time the source tree changed.

More broadly, though, we need to adjust our threat models to include potentially malicious maintainers. The two mitigations I've discussed here could address some of that - visibility and verifiability of source distribution, and reducing the attack surface presented by the build environment. There's going to be lots of other threats that come to light as we come to grips with this new threat model, though - we need to be careful not to focus too closely on the details of this particular attack.

A backdoor in xz

Posted Mar 31, 2024 14:06 UTC (Sun) by nix (subscriber, #2304) [Link] (5 responses)

> We want to avoid binary blobs in the source repository because we want the entire repository to be human readable, and therefore at least notionally subject to human review - it's not ridiculous to consider the practicality of the idea. Particularly for cases like this where it's a binary file format which obviously needs binary data as input for at least some of its test cases. It's definitely /possible/, and it may be desirable in the abstract, but how practical it'd be, how much value it'd add, and whether it'd be worth the disruption are the questions that need answering. Certainly it would mitigate against this particular attack, and raises the bar for any attack that wanted to hide a payload in plain sight, but it'd need careful consideration before being recommended as a matter of course.

I'd say that in the specific case of xz, it should be generating corrupted test files etc using *xz itself*, and then buggering them using existing tooling. If it had done that routinely it would be obvious that a giant binary lump claiming to be a corrupted xz file was up to no good.

The hard part is testing *non*-corrupted files. You can't reliably construct *those* with the built xz, because if the built xz is buggy it's going to generate a messed-up file rather than the non-corrupted file that is desired (and you presumably want to detect cases where the compressor and decompressor are buggered-up in the same way). Not sure how to fix that: you can't use the xz already on the system because it might be similarly buggered-up...

A backdoor in xz

Posted Mar 31, 2024 19:05 UTC (Sun) by cesarb (subscriber, #6266) [Link] (4 responses)

In my opinion, it's the opposite: non-corrupted files could be described using a custom human-readable assembly-like language which represents the primitives allowed in the compression format (something like "LITERAL 03 AA BB CC TREE ..."), while corrupted files could be crazy files found by users which happened to cause older versions of the software to misbehave, and which were added as a regression test.

> You can't reliably construct *those* with the built xz, because if the built xz is buggy it's going to generate a messed-up file [...] and you presumably want to detect cases where the compressor and decompressor are buggered-up in the same way

That is easy to avoid, you just need to add an "expected hash" to check that the compressor generated the correct data for the decompressor to test. But that doesn't help when you want to test that the decompressor remains compatible with data generated by *older* versions of the compressor (which the current code no longer generates), or even alternative compressor implementations. There's a lot of flexibility in compressor output, so it's not unusual for it to change.

A backdoor in xz

Posted Mar 31, 2024 19:41 UTC (Sun) by nix (subscriber, #2304) [Link]

Yeah, solving this is hard.

In a similar area, for a long time we tried to make the libctf and ld.ctf testsuites in binutils work the same way the DWARF ones mostly do: describe a binary CTF dict via a .S file and just assemble it. But a mixture of there being no cross-arch-portable way to represent arbitrary binary data in .S files (.long, .byte4 etc, are all arch-dependent) and a desire to always test the latest CTF format except in a few error cases meant that nearly all of the tests end up compiling a .c file with -gctf just to get its CTF, then running the tests on that. It turned out to be much nicer to work with that, and much easier to see what the tests were actually *doing* as well: .S files are a pretty opaque way to describe a test input! (For corrupted CTF tests, the same problem arises, though at only a few dozen bytes each they're hardly likely to be malicious. There's just no room.)

A backdoor in xz

Posted Apr 1, 2024 6:52 UTC (Mon) by himi (subscriber, #340) [Link] (2 responses)

> In my opinion, it's the opposite: non-corrupted files could be described using a custom human-readable assembly-like language which represents the primitives allowed in the compression format (something like "LITERAL 03 AA BB CC TREE ..."), while corrupted files could be crazy files found by users which happened to cause older versions of the software to misbehave, and which were added as a regression test.

This is mostly what I was thinking of - have a human-readable language designed to specify the binary formats, and use that to generate the test data. It would work for both good and bad test data, though - rather than simply copying "known bad" test files for testing, you'd pull them apart and write a spec for the test case that generated the same kind of bad data. Doing that would actually specify the badness in great detail, far more so than simply having a binary test file that happens to break things; it could also help drive highly targeted fuzzing whenever particular failures were identified.

The biggest problem I see is that for something like a compression format the difference in complexity between a specification of the binary format and it's possible contents and an actual implementation of the compression algorithm probably isn't all that big, and the specification is certainly going to be subject to as many bugs as the code it's supposed to be testing. This is why I'd hesitate to suggest trying to make this sort of thing into any kind of generalised "best practise" - there are probably going to be too many cases where the payoff just isn't worth the effort required.

That said, a compression algorithm and its on-disk format are probably just about the worst case scenario - I expect a lot of software that reads or writes a binary format would have much less difficulty creating this kind of test data spec. And in a more generalised sense the idea of generating test data rather than simply embedding it in the repository is something that's probably worth encouraging, along with tooling that would support it . . .

A backdoor in xz

Posted Apr 1, 2024 12:48 UTC (Mon) by pizza (subscriber, #46) [Link] (1 responses)

> This is mostly what I was thinking of - have a human-readable language designed to specify the binary formats, and use that to generate the test data.

So... you have this human-readable language generate a malicious file that contains the payload for an exploit. What have you gained here?

The problem is that the binary data is "hostile", not "how the binary data was generated".

A backdoor in xz

Posted Apr 2, 2024 3:16 UTC (Tue) by himi (subscriber, #340) [Link]

If we can't simply trust that the contents of the repository aren't malicious, we need to be able to independently verify that they're not malicious. The point of a human-readable specification of the binary data is to try and make it possible for a human to read it and say "the binary version of this spec won't be malicious". Is that possible? In the general case I don't think so, but I'm pretty sure there are going to be a good number of cases where it /is/ possible, for someone with the requisite knowledge of the format. I'm suggesting that where it's possible, it might be worth trying to do, to mitigate against at least some of the risks posed by malicious binary blobs in source repositories.

As I've said, even if this idea might work in some cases I suspect it wouldn't be viable for xz or similar, simply because of the nature of compression algorithms. But if you can make "has undefined binary files as part of the test data set" into a code smell that gets people to take a closer look then you're raising the bar for sneaking a malicious payload into a repository.