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

xz: Disable ifunc to fix Issue 60259. #10667

Merged
merged 1 commit into from Jul 7, 2023

Conversation

JiaT75
Copy link
Contributor

@JiaT75 JiaT75 commented Jul 7, 2023

Indirect function support was added to xz on machines that support it for function dispatching. ifunc is not compatible with -fsanitize=address, so this should be disabled for fuzzing builds.

Indirect function support was added to xz on machines that support it
for function dispatching. ifunc is not compatible with
-fsanitize=address, so this should be disabled for fuzzing builds.
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

JiaT75 is either the primary contact or is in the CCs list of projects/xz.
JiaT75 has previously contributed to projects/xz. The previous PR was #9960

Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tl;dr This patch did not prevent OSS-Fuzz from finding the backdoor, OSS-Fuzz is incapable of finding the backdoor, this patch is not suspicious, and OSS-Fuzz is not meant to find bugs when users do not want them found.

UPDATE:
There is no backdoor in this patch.
This repo provides a testing service to open source developers to help them find bugs in their code.
We allow developers to control 100% of what gets tested by our service. They own the code they submit to us, our reviews only mean we verified this is a project maintainer. Unfortunately we were told by the other XZ maintainer that "Jia Tan" was a maintainer.
If users don't want code tested, an automated tool like this can't stop them and was never intended to.

OSS-Fuzz is not impacted by the backdoor, because the backdoor never ran on OSS-Fuzz. In addition, OSS-Fuzz runs code in a sandboxed environment so malicious developers/projects cannot harm OSS-Fuzz.

UPDATE 2:
I wanted to highlight the half-dozen reasons why the backdoor would never have been discovered by OSS-Fuzz. It's totally implausible that it would have been discovered here.

  1. This pull request fixed a build breakage caused by a compiler issue. The compiler issue was never fixed, so without this pull request, OSS-Fuzz would have never fuzzed an XZ version after June 2023, the backdoor was added in March 2024.
  2. The XZ backdoor only built in the tarballs included in the 5.6.0 and 5.6.1 branches and not in the master branch of the git repo that OSS-Fuzz was cloning.
  3. The XZ backdoor was only built when using build options for DEB or RPM packages. OSS-Fuzz was using neither: https://openwall.com/lists/oss-security/2024/03/29/4#:~:text=Running%20as%20part%20of%20a%20debian%20or%20RPM%20package%20build
    (this analysis is from the person who discovered the backdoor).
  4. The XZ backdoor only ran when argv[0] was /usr/sbin/sshd This would never happen in OSS-Fuzz, only in a real environment https://openwall.com/lists/oss-security/2024/03/29/4#:~:text=argv%5B0%5D%20needs%20to%20be%20/usr/sbin/sshd
  5. Nothing OSS-Fuzz does is secret, all the tests and code are public. This was a well planned attack and if the backdoor was going to be run on OSS-Fuzz (which it was not), it is very likely "Jia Tan" would have made sure the tests passed. This is very similar to malware writers who test their malware against antivirus until it is undetectable. OSS-Fuzz is in the business of finding accidental bugs not intentional ones.
  6. OSS-Fuzz does not detect backdoors and it never has detected one. The backdoor's buginess did lead to it being discovered by Valgrind. However OSS-Fuzz's bug detectors would not have found the same issues. OSS-Fuzz uses ASAN to detect memory corruption bugs. ASAN requires instrumentation of a binary to detect bugs in the binary. This is not the case with Valgrind. The backdoor was not instrumented with ASAN, so OSS-Fuzz would not be able to detect bugs in the backdoor (in this case), though someone using Valgrind could detect them.

To summarize: the backdoor never ran on OSS-Fuzz and OSS-Fuzz would not detect the backdoor if somehow it magically ran on OSS-Fuzz.

UPDATE 3:
This patch fixes an actual false positive in ASAN.
If you change the old XZ dockerfile to check out the XZ source at the time of this PR, you can see how the fuzzers segfault immediately like all other users of ASAN and ifuncs.
ASAN's false positives can still be reproduced using this test file in OSS-Fuzz (and possibly other clang-15 builds)

void crunch1(void) {}
void crunch2(void) {}

__attribute__((__ifunc__("resolve_crunch")))
void crunch(void);

// __attribute__((__used__,__no_sanitize_address__))
static __typeof__(crunch)* resolve_crunch(void)
{
	__builtin_cpu_init();
	return __builtin_cpu_supports("avx") ? crunch1 : crunch2;
}

int main(void)
{
	crunch();
}

@jonathanmetzman jonathanmetzman merged commit d2e42b2 into google:master Jul 7, 2023
16 checks passed
@marekr
Copy link

marekr commented Mar 29, 2024

Hmm, should this be reverted because it's part of hiding the exploit for CVE-2024-3094 ?

https://www.openwall.com/lists/oss-security/2024/03/29/4

@jonathanmetzman
Copy link
Contributor

jonathanmetzman commented Mar 29, 2024

In hindsight, this does not "look good to me" :-)
We've disabled the projects for now, but will try to explore how this PR could have prevented discovery of this issue.

@rugk
Copy link

rugk commented Mar 29, 2024

Also, what issue number is that, which was apparently (tried to be) referenced? #60259 does not exist. (I also checked the xz project, it's 404 there too: https://github.com/tukaani-project/xz/issues/60259)
You can see that in my text, because it generates no link:
grafik

Also referencing in issue titles does not work on GitHub. In hindsight, this could be deliberate, because:

  1. This (i.e placing issue numbers in GitHub titles) is often done by new (GitHub) contributors, but the contributor here obviously is not new to how GitHub works.
  2. It obviously solves the otherwise obvious "red herring" of "wait, the issue number is not linked/blue?? what's up?" preventing reviewers from asking nasty questions…
  3. It gives credibility, as it looks as if an issue has been fixed/was there… which it apparently is not?

@iProdigy
Copy link

@rugk, the referenced issue is https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=60259

@rugk
Copy link

rugk commented Mar 29, 2024

Ah thanks, should have followed Occham's razor that has "it's probable some other issue tracker" as a more probable solution… 😉

@R00tkitSMM
Copy link

R00tkitSMM commented Mar 30, 2024

Indirect function support was added to xz on machines that support it for function dispatching. ifunc is not compatible with -fsanitize=address, so this should be disabled for fuzzing builds.

has been resolved this incompatibly? I just simply tested ifunc with -fsanitize=address and it was compatible.

@creikey
Copy link

creikey commented Mar 30, 2024

https://youtu.be/ypZ9JvUqaao

@krishjainx
Copy link

https://youtu.be/ypZ9JvUqaao

@creikey Jonathan makes some fair points, but this isn't about open vs closed source. Before "Jia Tan"'s role, the original maintainer cited mental health for limited involvement.

Comparing an underfunded open-source project to Windows is unfair. You'd need to compare under-resourced open source to small closed-source projects, which are plentiful across platforms.

A better comparison is the Linux kernel (or Google's OSS-Fuzz) vs Windows, though their development processes differ. The Linux kernel has oversight from major tech companies contributing financially and with human capital.

For Windows, the centralized security team may get sidelined as Microsoft prioritizes cloud/Azure and AI, allowing potential backdoors.

The lesson that we should learn as a community is more to secure software supply chain security holistically, auditing build systems beyond just source code. Like the SolarWinds breach where attackers modified software updates for SolarWinds closed-source monitoring software offering.

So, it's not a battle of open vs closed source software, and that's a waste of our energy, but of implementing better software supply chain security and being more vigilant. Thanks to all who helped with CVE-2024-3094.

@exikyut
Copy link

exikyut commented Mar 30, 2024

This is really interesting, it would be cool to have some insightful discussion about the situation as it stands right now.

Perusing through the build logs linked from https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=60259, we see that AFL crashes before even getting started

Step #4 - "build-check-afl-address-x86_64": �[1;91m[-] �[0mWhoops, the target binary crashed suddenly, before receiving any input
Step #4 - "build-check-afl-address-x86_64":     from the fuzzer! You can try the following:
Step #4 - "build-check-afl-address-x86_64": 
Step #4 - "build-check-afl-address-x86_64":     - The target binary crashes because necessary runtime conditions it needs
Step #4 - "build-check-afl-address-x86_64":       are not met. Try to:
Step #4 - "build-check-afl-address-x86_64":       1. Run again with AFL_DEBUG=1 set and check the output of the target
Step #4 - "build-check-afl-address-x86_64":          binary for clues.
Step #4 - "build-check-afl-address-x86_64":       2. Run again with AFL_DEBUG=1 and 'ulimit -c unlimited' and analyze the
Step #4 - "build-check-afl-address-x86_64":          generated core dump.
Step #4 - "build-check-afl-address-x86_64": 
Step #4 - "build-check-afl-address-x86_64":     - Possibly the target requires a huge coverage map and has CTORS.
Step #4 - "build-check-afl-address-x86_64":       Retry with setting AFL_MAP_SIZE=10000000.
Step #4 - "build-check-afl-address-x86_64": 
Step #4 - "build-check-afl-address-x86_64":     - Less likely, there is a horrible bug in the fuzzer. If other options
Step #4 - "build-check-afl-address-x86_64":       fail, poke the Awesome Fuzzing Discord for troubleshooting tips.
Step #4 - "build-check-afl-address-x86_64": ��)B�[?25h�[0m�[1;91m
Step #4 - "build-check-afl-address-x86_64": [-] PROGRAM ABORT : �[0mFork server crashed with signal 11�[1;91m
Step #4 - "build-check-afl-address-x86_64":          Location : �[0mafl_fsrv_start(), src/afl-forkserver.c:1281

and this is presumably because the exploit code is instantly falling victim to AFL's architectural purpose and design 🥳 and the way it sets up the execution environment.

If I'm understanding correctly, it would seem that the reality is that -fsanitize=address is not the legitimate cause of the reported failures and that this is a deception - so perhaps the path forward might be for this flag to be reenabled, indeed and precisely to improve Security And Resilience Against The Unknown™?

This comment is mostly commentary born of curiosity. I'm practically a moderately technically minded layperson here given all the dimensions. Obviously this detail will be reasoned through and a decision will be made eventually, and maybe it's not the highest priority right now. But these threads are being read by a lot of people, so perhaps a cursory explanation with a detail or two may prove insightful to some.

@marekr
Copy link

marekr commented Mar 30, 2024

The lesson that we should learn as a community is more to secure software supply chain security holistically, auditing build systems beyond just source code.

https://www.softwaremaxims.com/blog/not-a-supplier

@DragD
Copy link

DragD commented Mar 30, 2024

https://www.youtube.com/watch?v=jqjtNDtbDNI

Issaminu

This comment was marked as spam.

@goss-gowtham
Copy link

It's all come down to human conscience in the world of automated tests, builds and all the fancy AI models. Andrews Freund needs to be celebrated!

@shakefu
Copy link

shakefu commented Mar 30, 2024

Imagine a world where every company using "xz" somewhere in their infrastructure or code just contributed $1 a year to the OSS project that they depended upon so the overworked and unpaid maintainer didn't feel the need to abandon their work...

@ThiefMaster
Copy link

The sad truth is that in most companies it's much easier to spend $10k on some - maybe even crappy - commercial software than donate $1, $10 or $100 to some developers whose software you use all the time...

@krishjainx
Copy link

Imagine a world where every company using "xz" somewhere in their infrastructure or code just contributed $1 a year to the OSS project that they depended upon so the overworked and unpaid maintainer didn't feel the need to abandon their work...

While I understand the sentiment behind wanting companies to contribute to open source projects they rely on, the scalability issue goes beyond just relying on voluntary compliance. With thousands of small open source projects forming the backbone of larger systems, the administrative burden of tracking and fairly distributing even nominal contributions becomes impractical. Moreover, the value that different users extract from these open source components can vary greatly, suggesting that a more nuanced approach to funding is necessary to address the wide range of dependencies and their criticality to different projects and companies. This complexity makes a flat-rate, voluntary contribution model unfeasible for effectively supporting the open-source ecosystem.

Also, can we please stop commenting "lgtm" on this pull request? It isn't a good look, and most definitely, the PR doesn't "look good" either. Let's maintain a professional tone

@cmuller
Copy link

cmuller commented Mar 30, 2024

Imagine a world where every company using "xz" somewhere in their infrastructure or code just contributed $1 a year to the OSS project that they depended upon so the overworked and unpaid maintainer didn't feel the need to abandon their work...

https://imgs.xkcd.com/comics/dependency.png

@Starttoaster
Copy link

Starttoaster commented Mar 30, 2024

Imagine a world where every company using "xz" somewhere in their infrastructure or code just contributed $1 a year to the OSS project that they depended upon so the overworked and unpaid maintainer didn't feel the need to abandon their work...

Legitimate question here: has anybody compiled a list of OSS projects that users of X thing are likely to actually depend on? I imagine just being a user of Ubuntu means giving $1 to hundreds, if not thousands, of different organizations and people. A company needs to vet every individual and company they send money to, which you could argue they should be doing anyway with their software dependencies. But many of the things that come in a standard linux distro install are often somewhat abstract to the company. They miss the trees for the forest (to reverse a common idiom.)

Just to be clear, I'm not arguing against the sentiment of this. I'm all for it... With the caveat of it being a rather tall order just taking on the discovery part of it. But maybe a discovery tool exists and I'm just ignorant to it :)

@ljharb
Copy link
Contributor

ljharb commented Mar 30, 2024

@Starttoaster that's what tidelift and thanks.dev are for.

@Starttoaster
Copy link

Starttoaster commented Mar 30, 2024

@Starttoaster that's what tidelift and thanks.dev are for.

Looking specifically at thanks.dev, that seems to be useful for dependencies that the code your company distributes depends on, and doesn't seem like it really accounts for the case of: my company's infrastructure team stood up some Ubuntu servers, what all is in there that makes Ubuntu possible. Is that correct? Because that is what I'm saying I don't know exists. This is useful too though, thanks. Tidelift I need to take a closer look at because they don't seem to have a public demo, have to reach out to their marketing and end up on a marketing list. Tidelift might also be prohibitively expensive even if it does the thing I'm asking for. They don't publicize their pricing, it's all covered up with talks to their marketing team. So if I, an individual (not an enterprise), want to spread some love around to all the projects that make Ubuntu, or Proxmox, or any other tool I use possible, it seems like a somewhat insurmountable task without a discovery tool for those. I could give money directly to Canonical, but what are the odds that they send over some money to the xz project?

@cwegener
Copy link

https://youtu.be/ypZ9JvUqaao

Jonathan almost always gets the larger point right, but then completely fumbles on the details with his opinions.

@google google deleted a comment from playervalley Mar 31, 2024
@google google deleted a comment from atraxsrc Mar 31, 2024
@google google deleted a comment Mar 31, 2024
@google google deleted a comment from nt92 Mar 31, 2024
@google google deleted a comment from hbrooks Mar 31, 2024
@google google deleted a comment from Speculate7348 Mar 31, 2024
@google google locked as off-topic and limited conversation to collaborators Mar 31, 2024
@jonathanmetzman
Copy link
Contributor

There's useful conversation going on here but I think it's off topic for OSS-Fuzz and the pull request is becoming a magnet for spam, so I have to lock this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet