Problem: you need to be a "contributor" to the repo for your PR to trigger workflows without someone approving them first.

So: "We needed to be a contributor to the PyTorch repository to execute workflows, but we didn’t feel like spending time adding features to PyTorch. Instead, we found a typo in a markdown file and submitted a fix."

I really don't like this aspect of GitHub that people who have submitted a typo fix gain additional privileges on the repo by default. That's something GitHub can fix: I think "this user gets to trigger PRs without approval in the future" should be an active button repo administrators need to click, maybe in the PR flow there could be "Approve this run" and "Approve this run and all future runs by user X" buttons.

^So you could reject a PR that tries to add a bitcoin-mining workflow only to have it start bitcoin-mining on close. Or leaking secrets, releasing nonsense, etc. (I don't recall if non-/contributor status was also a factor. I think it was also ignoring that check.)

What privileges do they gain? Anything other than the one you mentioned?

This was definitely not obvious to me, so thanks for bringing it up!

NONE, CONTRIBUTOR, COLLABORATOR, MEMBER, OWNER

There are some cases where people use checks for that as part of gating for workflows that run on pull_request_target/issue_comment, but might confuse contributor and collaborator (which requires explicitly adding someone to the repository). Ultimately this is a misconfiguration on part of the maintainer but another example where fixing a typo can play a part in an attack.

Is the issue here that a self-hosted runner was needed for some hardware tests?

* pre-merge builds on PRs. These should not have privileges, but making the distinction between the two cases requires a lot of care.

* official builds of "master" or a feature branch from the main repo. These "need" privileges to upload the resulting artifacts somewhere. Of course, if all it did was wake up some daemon elsewhere, which could download straight from the CI in a verified way based on the CI's notion of the job name, it would be secure without privileges, but most CI systems don't want to preserve huge artifacts, and maintaining the separate daemon is also annoying.

Only because pypi removed support for gpg and their only "signature" now basically is "was uploaded from a gh action".

Otherwise a maintainer could do it offline without needing to give microsoft total access to uploading to pypi.

Without that there would be no need to have an action to do an upload. It could be comfortably and safely done offline.

Builds off of main get the secrets, pull requests from randos don’t.

And public repos don’t pay for CI on GitHub.

Not rocket science, people.

The smugness and overconfidence of someone who's about to be pwned?

Here's a similar mistake on an OSS repo a company I worked for made: https://goteleport.com/blog/hack-via-pull-request/

In several of our other operations, not just PyTorch, we leveraged workflow_dispatch to steal a PAT from another workflows. Developers tend to over-provision PATs so often. More often than not we'd end up with a PAT that has all scopes checked and org admin permissions. With that one could clean out all of the secrets from an organization in minutes using automated tools such as https://github.com/praetorian-inc/gato.

The key thing with a non-ephemeral runner is that (after obtaining persistence) you can grab the GITHUB_TOKEN from a subsequent non-fork PR build or a build on another trigger, which will have write permissions unless restricted by the repository maintainers.

When there are no side effects and no in-container secrets and the hosting is free or reasonably limited to prevent abusers, ideally yes.

Outside that, heck no, that'd be crazy. You're allowing randos to run arbitrary code on your budget. Locking it down until it's reviewed is like step 1, they can validate locally until then.

The issue is that now the pull requests don't get tested at all. I have to manually, locally, get all the commits, make a branch on the main repository with them, and then the actions run.

My prior is that GitHub is pretty competent in understanding malicious PRs to open source repos, and wouldn’t penalize the repo owner without other evidence of wrongdoing.

Afaict, a huge portion of this attack came from persistence on the self-hosted runner.

Absent that, they would have needed a container jailbreak as well, which substantially ups the difficulty.

And if a repo is running

https://docs.github.com/en/actions/hosting-your-own-runners/...

[1]: https://runs-on.com

1. ephemeral + zero implicit trust (2) https://blog.openziti.io/my-intern-assignment-call-a-dark-we...

2. zero implicit trust: https://github.com/openziti/ziti-webhook-action

(1) disclosure, maintainer (2) zero implicit trust in this case = no open inbound ports on underlay; need to access via app-specific overlay which requires strong identity, authN, authZ

Also worth reading a similar example: https://blog.cloudflare.com/cloudflares-handling-of-an-rce-v...

Also, let me recommend our bug bounty program: https://github.com/ClickHouse/ClickHouse/issues/38986 It sounds easy - pick your favorite fuzzer, find a segfault (it should be easy because C++ isn't a memory-safe language), and get your paycheck.

Here is an example: https://github.com/ClickHouse/ClickHouse/pull/58794/files

The right way is to:

- always pin versions of all packages; - this includes OS package repositories, Docker repositories, as well as pip, npm, cargo, and others; - never download anything from the master/main or other branches; specify commit sha; - ideally, copy all Docker images to our own private registry; - ideally, calculate hashes after download and compare them with what was before; - frankly speaking, if CI runs air-gapped, it would be much better...

Now it's just anyone with write access to Aliyun's repository. :) (p.s. GitHub doesn't consider this a security issue).

Really a misinformed comment.

For starters Ubuntu is for the most part a snapshot of Debian sid, so except for a few cases it will not have more modern versions.

The python packaging team is really hard working… In most cases stuff that doesn't get updated immediately is because it breaks something or depends on something new that isn't packaged yet.

Please stop demeaning the work of others when you seem to not know that it even happens at all.

The balance there is overwhelmingly in favor of usability and having new tools (hence the 1 week deployment cadence). Maybe there is some process they have to go over everything before it makes it into the production pool, but that’s quite an undertaking to perform properly every week.

Producers:

* Self-hosted infrastructure should not be running anonymous code. PRs should be reviewed before code executes on your infrastructure. Potentially should be a GitHub default when using self-hosted runners?

* Permissions for workflows and tokens should be minimal and fine-grained. "permissions: read-all" should be your default when creating a new workflow. Prevents lateral movement via modifying workflow code.

* Self-hosted infrastructure should be isolated and ephemeral, persistence was key for lateral movement with this attack.

Consumers:

* Use a lock file with pinned hashes, either --require-hashes or poetry/pipfile

* Review the diff of the file getting installed, not the GitHub source code. This will get easier when build provenance becomes a feature of PyPI.

* If your organization is large enough, consider mirroring PyPI with approved releases so the manual review effort can be amortized.

* More coming in this space for Python, like third-party attestations about malware, provenance, build reproducibility, etc. Stay tuned! :)

Half the point of self hosting is to reuse cached resources.

Very often local tests results can't be trusted specially for projects with architecture level codes like pytorch. Before merging the test results needs to be checked. And it doesn't require just PR review to be safe, it requires review of all the commits as the contributor is making the changes to fix the testcase. Even if we assume that the maintainer will review each of the commit within a day, it could take weeks or months for the contributor to fix the failing testcase with this and maintainer to be looking at the same PR everyday.

> We used our C2 repository to execute the pwd && ls && /home && ip a command on the runner labeled “jenkins-worker-rocm-amd-34”, confirming stable C2 and remote code execution. We also ran sudo -l to confirm we had root access.

While it's not clear was it curated list of commands or just ALL, I assume the latter and that makes me feel no system administrator was involved into that pipelines setup - those guys are quite allergic to giving sudo/root access at all

The same design flaws in PyTorch that make it an interest attack vector (which in a very, very small way I had a part in, I also got this wrong) are the same design flaws that basically dictate that’s it’s more like an API than an implementation now: and those flaws are all the ways it’s too big to easily audit or port.

It’s a very good UI to accelerated computing, and I suspect it’s the “last one” for a long time.

I hope the next mainstream implementation will be TinyGrad, I fear it’ll be MLX, and I’d settle for JAX, but it won’t be PyTorch per se.

Also reverse props to the meta bug bounty program manager for not understanding the finding initially. I know it's difficult managing a program but it's not an excuse to brush something like this off.

How do you know?

i wonder about the over-dependence on third party packages and modules

imagine the author of 'is-odd' injects a trojan there

what are you gonna do?

C has this solved but 'vendoring' is not as fast as this approach

It hasn't… but C developers are much more careful about adding a dependency than js/python/rust/go developers. Mostly because adding a dependency in C is more annoying. In those languages it's just about adding one line.

Of course if you use a distribution and it's a famous library, it's to add a line as well. But then there is the filter of the distribution. Which would work for any language, but most developers vendor everything instead.

Compiled binaries are harder, you might consider compiling them from source and comparing the output. This is where build reproducibility comes in to play.

There's a lot more coming in the Python packaging security space that'll make this easier and just safer in general. Stay tuned :)

The issue is, for any given implementation, similar attacks could still happen, and the package repos are still probably vulnerable.

"Reflections on Trusting Trust" was a demonstration of this in the C ecosystem long before package managers were a thing.

These days it's practically a necessity for companies to shell out money to some sort of supply-chain protection software (Sonatype, Socket.dev etc.)

Once you've discovered a security hole, exploiting it to see how much access you can get is generally frowned upon.

I ended up having to reach out to someone senior I knew in the security org there to get them to swoop in and pick up the report, before it got any actual traction (I'd worked with that senior security engineer in a previous job).

For a long time Apple didn't have a bug bounty programme at all. This wasn't because they didn't care about security. It's because their own internal audits were generating enough reports to saturate the available capacity for bug fixing, so paying for more reports would have just duplicated work. Generally this is the pattern at big tech firms: you want to turn your internal security teams to a problem for a while before starting to pay out for a new class of bugs. But of course it's hard to descope a problem from bug bounties, it looks very bad.

However, not all organizations are happy to be contacted about security issues. Sometimes doing the right thing can still result in (threats of) legal repercussions.

https://arstechnica.com/tech-policy/2021/10/missouri-gov-cal...

The bug bounties also prefer seeing a working attack instead of theoretical reports. So not sure how they could have tested their attack in this situation without making actual changes.

It does seem like walking a precarious tight rope.

One should also use workflow approvals for external contributors.

Lockheed being listed makes me wonder if the FBI/CIA really will (further) step up on cybercrime, because you now have potential national security implications in a core supplier to multiple military branches.

It's true to the extent that the US does a lot more and broader military procurement in general, so Boeing gets a smaller piece of a much bigger pie. Wheras Airbus is getting a piece of most European projects as a member of one consortium or another, it's just a smaller pie.

I think supply chain attacks are not being taken very seriously. Think that people working, for example, in Python or JavaScript use pip or npm daily no matter if they work for a nuclear agency or your uncle's bar.

>So far, we’ve submitted over 20 bug bounty reports, raking in hundreds of thousands of dollars in bounties.

So I think this is part of a chain of bounties? Though that can still be argued to be a bit too low for how powerful this exploit could be :)

So, as you can hopefully see, it is a balancing act between all parties.

I could be wrong about this, but I've been loud about it around people who do a lot of this stuff and none of them have dunked on me in public. :)