4 ms·
can we start considering binary files committed to a repo, even as data for tests, to be a huge red flag, and that the binary files themselves should instead, t
by zzzeek 3y ago
can we start considering binary files committed to a repo, even as data for tests, to be a huge red flag, and that the binary files themselves should instead, to the greatest extent possible, be generated at testing time by source code that's stated as reviewable cleartext (though I think this might be very difficult for some situations). This would make it much harder (though of course we can never really say "impossible") to embed a substantial payload in this way.
when binary files are part of a test suite, they are typically trying to illustrate some element of the program being tested, in this case a file that was incorrectly xz-encoded. Binary files like these weren't typed by hand, they will always ultimately come from something plaintext source, modulo whatever "real world" data came in, like randomly generated numbers, audio or visual data, etc.
Here's an example! My own SQLAlchemy repository has a few binary files in it! https://github.com/sqlalchemy/sqlalchemy/blob/main/test/binary_data_one.dat https://github.com/sqlalchemy/sqlalchemy/blob/main/test/bina... oh noes. Why are those files there? well in this case I just wanted to test that I can send large binary BLOBs into the database driver and I was lazy. This is actually pretty dumb, the two binary files here add 35K of useless crap to the source, and I could just as easily generate this binary data on the fly using a two liner that spits out random bytes. Anyone could see that two liner and know that it isn't embedding a malicious payload.
If I wanted to generate a poorly formed .xz file, I'd illustrate source code that generates random data, runs it through .xz, then applies "corruption" to it, like zeroing out the high bit of every byte. The process by which this occurs would be all reviewable in source code.
Where I might be totally off here is if you're an image processing library and you want to test filters on an image, and you have the "before" and "after" images, or something similar for audio information, or other scientifically-generated real world datapoints that have some known meaning. That might be difficult to generate programmatically, and I guess even if said data were valid, payloads could be applied steganographically. So I don't know! But just like nobody would ever accept a PR that has a "curl https://some_rando_url/myfile.zip https://some_rando_url/myfile.zip" inside of it, we should not accept PRs that have non-cleartext binary data in them, or package them, without really vetting the contents of those binary files. The simple presence of a binary file in a PR can certainly be highlighted, github could put a huge red banner BINARY FILES IN THIS PR.
Downstream packagers for distros like Debian, Redhat etc. would ideally be similarly skeptical of new binary files that appear in the source downloads, and tooling can be applied to highlight the appearance of such files. Packagers would be on the hook to confirm the source of these binary files, or ensure they are deleted (even disabling tests if necessary) before the build process is performed.
- mnau 3y agoAny library that works with file formats needs binary files. A lot of them malformed (or output is slightly different than standard output), because they need to ensure they can work even with files generated by other programs. Bugs like ' I tried to load this file and it failed, but works in XYZ' are extremly common. These formats are often very complex and trying things like'zeroing out a high bit' doesn't cut it. Youvwould end up with binary code encoded in source. Edit: one of simple improvements github/other forges could do is show content of archives in a diff. The payload was hidden in a archive test file and it would be displayed in a diff instead of "binary file change, no idea what is in it"
- usefulcat 3y ago> Edit: one of simple improvements github/other forges could do is show content of archives in a diff. That works if the archives are valid as checked in, but not if they’re corrupted in a predictable way such that they can trivially be “un-corrupted” as needed, perhaps by something as simple as tr. Even if that’s not exactly what happened here, I think it’s pretty obvious how eminently doable that is, given the sophistication of so many aspects of this attack.
- klabb3 3y agoAbsolutely yes. As a rule of thumb, for sure. However, in reality the problem isn’t binary per se, but anything that’s too obfuscated or boilerplatey to audit manually. It could be magical strange strings or more commonly massive generated code (autoconf?). Those things should ideally not be checked in, imo, but at the very least, there needs to be an idempotent script that reproduces those same files and checks that they are derived correctly from other parts of the code. Ideally also in a separate dir that can be deleted cheaply and regenerated. For instance, in Go it’s quite common to generate files and check them in, for eg ORMs. If I run `rm -r ./gen` and then `go generate`, git will report a clean working dir if all is dandy. It’s trivial to add a CI hook for this to detect tampering. Similarly, you could check in the code that generates garbage with a seed, and thus have it be reproducible. You still need to audit the generators, but that’s acceptable.
- tremon 3y ago