9 ms·
Remote code execution in Homebrew by compromising the official Cask repository
- spenvo 5y agoHomebrew disclosure: https://brew.sh/2021/04/21/security-incident-disclosure/ https://brew.sh/2021/04/21/security-incident-disclosure/
- qbasic_forever 5y agoWow yikes, kudos to the great disclosure and fix process from homebrew and other involved folks. Github Actions seems like such an easy target--the workflows people build are complex, difficult to audit, and lightly or never tested at all except in prod. It's just a recipe for disaster and all these recent issues (see also: https://news.ycombinator.com/item?id=26908076 https://news.ycombinator.com/item?id=26908076) make me wonder if the Github flow model of a wild west of public pull requests just isn't compatible with how people use workflows and automation in practice. No one in their right mind would leave a Jenkins server open for anyone to trigger some workflows. It's silly that everyone effectively does that because they see some Github branding on a page and feel safer.
- oefrha 5y ago> No one in their right mind would leave a Jenkins server open for anyone to trigger some workflows. Open source projects have been doing exactly that for ages. Homebrew in particular had a Jenkins server hosted at bot.brew.sh running arbitrary pull requests since 2013 before finally fully migrating to GitHub Actions in 2020.
- alfiedotwtf 5y agoNot only this, but public Jenkins often have their shell open to the public!
- capableweb 5y ago> Open source projects have been doing exactly that for ages. I'm not sure it's that common. What's common is to allow people to browse the Jenkins instance, but disable anything to run PRs that changes the workflow/build itself from people who are not part of the organization/team that manages Jenkins.
- oefrha 5y agoYou don’t need to make changes to workflows to run arbitrary code. Projects usually run tests, and you can just put whatever you want to execute in the code being tested, or exploit other holes in the existing workflow. TFA does not make any changes to existing workflows, for instance. Also, you can disable workflow changes in GitHub Actions with the pull_request_target event, in which case it uses the workflow definition from the target branch.
- Master_Odin 5y agoThe pull_request_target event however allows for read/write access to the base repo and access to all of its secrets. It's not the event type you'd want to use whenever dealing with code that's incoming in a PR. See the big warning at https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request_target https://docs.github.com/en/actions/reference/events-that-tri....
- oefrha 5y agoThat’s not true at all. Unless you expose the token or secrets in steps where you run untrusted code, the token and secrets are safe. This is explicitly designed for untrusted pull requests on public repos, solving the problem of, say, adding labels with the write token, or deploying a preview build to Netlify, while keeping the secrets safe from arbitrary code execution.
- sneak 5y agoRunning arbitrary code from users in CI is one thing. Automatically merging code is another, which is what this bug exploited. The former can be made somewhat/reasonably safe. The latter probably can't.
- oefrha 5y agoYes, but that is not what this thread (GitHub Actions is somehow more vulnerable than Jenkins because no one exposes Jenkins?) is about.
- TreeInBuxton 5y agoThis is exactly why on a project I recently set up CI on, it is only run when a collaborator gives an approving review, or something is pushed directly to master. I saw far too much of an attack surface from Github actions, especially with MSBuild.
- surajs 5y agoHad to happen someday
- rubatuga 5y agoWhy do they even use a feature like automerge without approval? I feel like homebrew needs a solid dose of KISS (keep it simple, stupid)
- IshKebab 5y agoTo reduce the amount of effort by maintainers.
- esjeon 5y agoAutomerge is one thing, it's the implementation that relied on some pure text hack. Matching text is a hard problem, and regex is not a solution, but a mere tool that translates one problem to another. Actually, it only creates more problems: Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems
- blablabla123 5y ago> Why do they even use a feature like automerge I think so too. On the other hand that goes in line with all the CI/CD/CX craze - which involves commonly accepted complexity in its own way. > without approval Probably that also shows that the project might need more contributors. To me it's a core component of every macOS install but actually a voluntary project which is an unusual combination.
- stjo 5y agoI feel a lot less secure now knowing there is an automated pipeline glued with yaml and ruby, that takes a stranger's code and pushes it directly to my machine. I'm sure the homebrew team tried hard to make sure it is as simple and restrictive as possible. As they are doing it for free and without warranty, they are not obligated to match any security standards. I am grateful, but I still wish it is done differently. Maybe instead of armchair complaining on HN I should donate...
- aphextron 5y agoI'm not sure yaml and ruby have anything to do with it. This is a common risk with all centralized package management systems. It's ultimately a tradeoff of trusting a third party for the speed and simplicity. The alternative is building everything from source, or hashing your own binaries. If anything, this is on Apple for the inexplicable fact that they haven't implemented native package management in macOS yet.
- stjo 5y agoWell, if they didn’t made the trade off between security (manual pull request review) and easy of maintenance (automatic pull request merge), this would have happened. If instead of parsing git diffs with ruby they used libgit (or something equivalent in how much battle tested it is), this wouldn’t have happened. If Apple provided a more capable package manager, this wouldn’t have happened. If the millions of customers of homebrew (as am I) gave them even a fraction of the monetary resources they need to build such a massive project, this wouldn’t have happened. As you say, there is no single responsible party for the vulnerability. But it is an interesting case too ponder about. If instead of responsibly disclosed, a malicious party decided to use it, this could’ve been very ugly. I hope it gets noticed by the community and better measures are taken.
- thu2111 5y agoWell, it's also just bad engineering tbh. What they're trying to do here is take a workflow intended for humans to make arbitrary code changes and other humans to review those changes, then restrict it to be a workflow for machines to update two keys in a database. Why not just introduce a real API for people to submit new versions instead of hacking one together from tools never meant for this use case? Honestly I use brew and love it but the decision to make everything git and ruby is super hacky. And fundamental architectural decisions like that aren't something you can fix with donations.
- IshKebab 5y agoThis is a good lesson in why you should almost never use regex for parsing and validation.
- ahel 5y agoWhat would you have used in this case?
- IshKebab 5y agoI would have used a library that parsed the diff properly - or better yet one that applied the diff algorithm natively without going through a patch file format. E.g. something like this in Rust: https://github.com/utkarshkukreti/diff.rs https://github.com/utkarshkukreti/diff.rs (there are similar libraries for other languages).
- sneak 5y agoThe case against using homebrew mounts daily. Their poor decisions (regarding all of engineering, community/social, and privacy) are numerous enough for a non-listicle blog post at this point. I switched to using Nix on macOS to manage packages, and it's fantastic. The nix concept in general is wonderful, and there's a lot of work put in to making it work well on the mac. I haven't missed `brew` at all.
- b0afc375b5 5y agoThanks for mentioning nix. I didn't know it had a macOS variant. I've been meaning to learn the whole nix ecosystem but hadn't found the energy yet.
- sheenobu 5y agoNix pills might be a good start if you want to understand how all the pieces fit together. https://nixos.org/guides/nix-pills https://nixos.org/guides/nix-pills. There is nix, the language/package manager which can be used standalone; nixpkgs, the ports or homebrew like repo of packages that can be built/run on many systems including macOS; and nixos, the Linux+Nix distro that puts it all together.
- b0afc375b5 5y agoThanks. I've already installed nix/nix-darwin and tried installing wireshark, but it isn't working for some reason. I have some reading to do.
- sheenobu 5y agoI thought of Nix while reading this thread and I'm wondering what makes it unique here? As a daily NixOS user I get that it is better but I don't know the specifics. the nixpkgs rpeo is superficially similar to homebrew (lots of people submitting packages, running on github, automation around commits). What are the differences wrt to security? 1. It's language, Nix, is limited in scope? 2. No automated PR merge workflows (yet)? 3. Better community/engineering/security?
- tannhaeuser 5y agoWhat's the story with MacPorts these days? When I used MacOS I preferred it over alternatives such as brew and was under the impression it was used and had contributors from Apple as well.
- lloeki 5y agoMacPorts is fine (a coworker is using it daily). There's also pkgsrc and nixpkgs (I use the latter daily), which are great. That kind of vulnerability being a real possibility is one of the main drivers behind ArchMac, although I did contribute to Homebrew, I always found it to be way too complex and brittle for its own good (no offense, many of it is a design choice, it’s just my personal opinion and why I steer clear of it)
- rbanffy 5y agoThere is no story, really. It just works. Combined with virtualenvwrapper it's my daily driver. Most of my colleagues went with the pyenv route and suffer for it. In reality, there is no real reason why you couldn't have both (I do) and some things I install under brew, while others (python versions, for instance) I install under MacPorts.
- Isthatablackgsd 5y agoWait? you could have Brew and MacPorts in the same user? I read that they are impossible to co-exists due to how brew and macports handles the user folder? BTW I never tried this, I just got this Macbook Air M1 few months ago. Fearful that I will brick it.
- kergonath 5y agoMacports works fine. There is no major issue with it. It was more or less blessed by Apple at some point (using Apple infrastructure for hosting and having Apple employees contributing such as Jordan Hubbard), but I don’t think they are still involved now.
- zdw 5y ago
- christian008 5y agoI started doing all my development inside a virtual machine. UTM on the M1 chip running Ubuntu ARM works very well and has minimal impact on battery life. The VS Code remote extension makes development inside the VM feel like its local.
- deleted 5y ago[deleted]
- rbanffy 5y agoWhat I dislike in the VM approach is that the development machine and the host are somewhat disconnected and it's non-trivial for them to share files. Also, I'm concerned I'll write software with too many linuxisms that would render it incompatible with other Unixes (which are still used in far too many places). With my tooling, I even try hard to make it work on Windows, just in case.
- uzakov 5y ago> and it's non-trivial for them to share files. My approach is to have a separate dropbox and GitHub account for sharing, that both machines have access to.
- Labo333 5y agoThat's why I use Parallels Desktop, even though I would prefer to use open source software. They provide great file system integration on common OS plus a few other things like GPU graphics (I tried with windows 10 arm).
- ChrisLTD 5y agoSo you run a Linux box using Parallels and use VS Code or something on the Mac side to edit files? I'm interested in adopting a similar workflow, so your guidance would be appreciated.
- 0x0 5y agoIf you share $HOME with your VM (which is apparently the default in Parallels), you're not gaining much in terms of security. https://zerodayengineering.com/blog/dont-share-your-home.html https://zerodayengineering.com/blog/dont-share-your-home.htm...
- dave_aiello 5y agoI think this article, and the discussion about it, are outstanding. I've been a casual Homebrew user for several years. It helps me set up an environment where it's relatively easy to access and work on my projects that involve lightweight software development, as opposed to lightweight ops, which is probably my strength. Working solo, I don't have the bandwidth to understand the details of everything I touch. I truly appreciate that there are people in the community who like to examine deeper technical issues with package management and also have the communications skills to explain it so people like me can understand.
- devit 5y agoIt's pretty unbelievable that someone would do something so insane as creating a script that parses pull requests and automatically merges code from anyone that satisfies some condition. Any sane person would just have the script approve pull requests from trusted parties instead to achieve their goal. This in addition to the incompetent programmer who managed to write code that incorrectly parses git diffs. I think it's important to point out the identity of those people so that there are at least some reputational consequences for the catastrophic harm people like these cause. The main culprit of this, the person who due to their sheer incompetence and negligence put all Homebrew users at risk of total system compromise by authoring and committing the insane automerge code, seems to be "Markus Reiter", "reitermarkus" on GitHub, who claims to be affiliated with the University of Innsbruck in Austria. The person responsible for recklessly promoting code as a "Ruby library for parsing the unified diff format generated with git diff" despite the code not correctly performing that task is Andrew Olson, github user name "anolson", claiming to work at Kajabi in Blacksburg, VA. At fault is also whoever gave "reitermarkus" commit access to the Homebrew-cask repository, and the whole chain up to the Homebrew founder, although it's not clear to me whether it's possible to determine that from the GitHub interface. The Homebrew founder, who through their reckless assignment of commit rights to incompetent programmers is ultimately responsible for the catastrophe, is Max Howell, "mxcl" on Github claiming to be from Savannah, GA. According to the web, this person was not capable of inverting a binary tree during a Google interview (a task that any competent programmer can trivially perform), so maybe it's not surprising that he initiated such a catastrophe.
- williesleg 5y agoUMN Chinese Research again?
- salmo 5y agoIgnoring the whole homebrew side of it. The article itself is a pretty great read. It explains the technical issue well. But additionally it shows the experience of the author and their process to find and exploit the issue.
- tyingq 5y agoTook me a bit to figure out that it's using Ruby code like this to fool the surrounding CI scripts. "b/#{puts 'test';}" That happens to run the code within the braces, despite it all being enclosed in double quotes and preceded with a #. Can a Ruby person explain what's happening here? I understand it's working as designed. Edit: Ah, okay, so the interpolation is #{code here;} everything else is a just a string in quotes. Interesting that they use the # character to denote interpolation.
- iudqnolq 5y agoIt's string interpolation where you can provide an expression, not just a variable. > String interpolation is common in many programming languages which make heavy use of string representations of data, such as Apache Groovy, Kotlin, Perl, PHP, Python, Ruby, Scala, Swift, Tcl and most Unix shells. Two modes of literal expression are usually offered: one with interpolation enabled, the other without (termed raw string). Placeholders are usually represented by a bare or a named sigil (typically $ or %), e.g. $placeholder or %123. Expansion of the string usually occurs at run time. https://en.wikipedia.org/wiki/String_interpolation#%3A%7E%3Atext%3DString_interpolation_is_common_in%2CTcl_and_most_Unix_shells.?wprov=sfla1 https://en.wikipedia.org/wiki/String_interpolation#%3A%7E%3A...
- KMnO4 5y agoString interpolation. It can be used for good but it’s possible to exploit if the user picks the string. Many languages have exactly the same feature: Python: f”Price: {get_price()}” JavaScript: `Price: ${get_price()}` Notice how both Python and JS require more than just double quotes; you have to explicitly opt in with f-strings or template literals.
- tyingq 5y agoIt was Ruby's use of the # character for interpolation that was throwing me off. Apparently you can skip the braces too, if the # precedes some types of variables.
- burlesona 5y agoRuby is also opt-in in the sense that single quoted strings do not support interpolation, and while the language doesn’t care if you use single or double quotes, most linters will enforce single quotes as the default.
- ec109685 5y agoThis action by GitHub will help prevent this type of exploit, which seems like a good thing. Folks who haven’t merged a pull request before will need approvals to trigger an action: https://github.blog/2021-04-22-github-actions-update-helping-maintainers-combat-bad-actors/ https://github.blog/2021-04-22-github-actions-update-helping...
- haik90 5y agoI don't think this prevent this exploit because it mergered by bot (BrewTestBot in this case). An this exploit still working if any of contributor create malicious update
- theptip 5y ago> After that, I received I can’t add a test cask just for this but you could try to make a harmless modification to an existing cask perhaps? from the staff. Therefore, I chose a random cask and decided to make harmless changes. Yikes. And indeed this “harmless modification” did leak into a user-facing change. Seems like questionable judgement from the brew security team here. Surely they should have a staging/test cask for hackers to attack. The lack of concern at pointing a hacker to make a real change to an arbitrary cask is worrying.
- myolxid1 5y agoIt is likely that you'd have seen it anyway, as `brew update` loads casks whether or not installed. Note that this happened only to people who tapped the cask repo. Turning off the autoupdate is highly not recommended - you don't get security updates for the software you install and you may experience issues with Homebrew. Chances are you'd have run `brew update` before you wanted to install/upgrade packages. The best thing to happen here is for auto reviewing to be turned off and I'm glad for it as a daily user. Perhaps a much larger change (for Homebrew 4.0?) would be to avoid loading these .rb files on update, perhaps use an online cache/API like formulae.brew.sh.
- FabHK 5y ago(Weird, I posted that yesterday, thought there was duplicate detection... but good that it gets discussed now. https://news.ycombinator.com/item?id=26916854 https://news.ycombinator.com/item?id=26916854 )
- burlesona 5y agoThe big thing I see is that it was only three hours from security report receipt to primary mitigation in place. That’s a very quick turnaround for a volunteer open source project. Kudos to the Homebrew team for running a disclosure program to find risks like this, and for their speedy mitigation!
- dcow 5y agoThey were conducting the program (with help from HackerOne) so they were on standby.
- mikemcquaid 5y agoThanks for the kind words, they are surprisingly motivating (and rare). I was the person who responded to this at the weekend while my youngest kid was having a nap. I'm glad to see people recognise that this sort of turnaround on a volunteer-run project has a human cost and doesn't come automatically.
- deleted 5y ago[deleted]