9 ms·
Helm local code execution via a malicious chart
- TheDong 1y agoThat description seems really unclear, like how can `Chart.lock` be a symlink to a `.bashrc`? Is the vulnerability that you ship a chart with `Chart.lock -> ../.bashrc`, and then helm writes to `Chart.lock`? Why is the fix specific to Chart.lock (https://github.com/helm/helm/commit/76fdba4c8c2a4829a6b7abb48a08e51fd07fa0b3 https://github.com/helm/helm/commit/76fdba4c8c2a4829a6b7abb4...), wouldn't the fix be instead that "A chart cannot contain any symlinks outside of its root"?
- yelirekim 1y agoI think that there are "legitimate" use cases for symlinks that read from outside the root, which at this point are probably looked upon even less favorably. It's likely that making the change you're proposing would be backwards incompatible. I agree that it's not clearly explained why this isn't a concern though. A cursory search for other instances of os.WriteFile doesn't seem to surface any thorough controls... edit: ok actually it looks like the lockfile is special because it's the only instance of helm itself directly writing a file on behalf of a package consumer
- TheDong 1y agoWhat use-case? If you have a chart that has `deploy.yaml` symlinked to `/home/john/testcharts/redis/deploy.yaml`, that chart is clearly not going to work on anyone's machine except john's, so that chart is useless on anyone else's machine. If you're saying the use-case is for charts that aren't distributed, well, I'm saying we should ban all symlinks on distribution (downloading and unpacking a chart should fail if it has symlinks outside of the root), and I just can't imagine any use-case where a distributed chart with external symlinks makes sense. If this whole thing is about charts that aren't distributed, but local to some developer's machine, well, in that case who cares if the developer can pwn themselves by typing "ln -s ~/.bashrc Chart.lock", they could have just pwned themselves by typing "bash" even more quickly.
- yelirekim 1y agoYa, I mean, I put "legitimate" in quotes for a reason. I think most people agree with you. This has been a thing that they've been aware of and struggling with for a while. https://helm.sh/blog/2019-10-30-helm-symlink-security-notice/ https://helm.sh/blog/2019-10-30-helm-symlink-security-notice... Smattering an --allow-symlinks flag all over their commands seems to be the least inelegant way to handle this while still giving users an easy way to maintain compatibility. Maybe they'll come around to it after this.
- nijave 1y agoI have use cases for linking Terraform lock files to keep various deployments/modules on consistent versions. I could see there being a use case for symlinking Chart.lock files although usually that's limited to an internal implementation and not something a general purpose chart would probably ship i.e. you have 3 different charts that all depends on `cache`, `load balancer` and `database` charts and you want to only ever have 1 version deployed of those subcharts so you want the parent chart locks linked
- sugarpimpdorsey 1y agoIf we're being honest, YAML is one of the dumbest ideas of the last 20 years to have proliferated. How we got from XML to here I cannot comprehend. This is not the first RCE involving YAML and it won't be the last.
- szszrk 1y agoThat was not RCE. It's not in yaml, it's in Helm's logic. But glad you vented, I guess.
- ChocolateGod 1y agoWhy we settled on a file format that relies on invisible characters I'll never know.
- imiric 1y agoYou use invisible characters whenever you press Enter or Space. If you're referring to Tab, many of the most popular programming languages like Go and Python use them as part of their syntax. The reason YAML was popularized is because it was a response to XML which isn't user friendly to write. It's unfortunate that the spec got so convoluted, and uses a lot of implicit behavior, but I'd rather write YAML than XML, JSON or TOML for things like configuration files. Nowadays there might be better alternatives, but YAML is the de facto standard. It's also unfortunate that YAML got abused by people who wanted to turn it into a DSL, so we ended up with thousands of lines of Ansible playbooks, CI workflows, and Helm charts, but here we are.
- drysart 1y agoIt's unfortunate, but inevitable. Every structured text data format that sees widespread use, given enough time, will eventually be turned into a DSL.
- cluckindan 1y agoIn fact, once a structured text format is used as a data source for any process, it has already become a DSL.
- agys 1y agoFor a moment I thought it was about the synth…! https://tytel.org/helm/ https://tytel.org/helm/
- qxfys 1y agoWondering how this kind of thing can be automatically discovered by an LLM. Anyone have any experience?
- Sjoerd 1y agoWhat is the attack scenario here? Where are the security boundaries? How does the attacker gets their repository with a symlink in it to the victim? Is Helm typically run as a privileged user? How would this work? And why doesn't the vulnerability description give answers to these questions?
- porridgeraisin 1y ago[dead]
- xyst 1y agoQuestions like this make me wonder if "hacker" news needs a rebranding. Basic tech news? Capitalist news? Vulture Capitalist news?
- deathanatos 1y ago> What is the attack scenario here? Given the details in the article, I think even something as simple a templating a chart from a repository might be vuln., but it likely depends on a lot of exact specifics. > Where are the security boundaries? I expect templating does not result in LCE. > How does the attacker gets their repository with a symlink in it to the victim? The attacker owns the repository. They can serve whatever maliciousness in it they want. But should templating a malicious chart result in LCE? > Is Helm typically run as a privileged user? Enough so, yes, because the rendered result is often pushed to a k8s cluster. "Privileged" here might not be "root", but it might be "this user has k8s API access". Imagine, e.g., that the attacker's LCE here might be "push ~/.kube to attacker". > And why doesn't the vulnerability description give answers to these questions? Familiarity with the tools involved is an normal assumption.
- yelirekim 1y agoThe original vulnerability description is not worded very well, here's my understanding of what's going on: 1. Attacker crafts a malicious Chart.yaml containing arbitrary code 2. Replaces Chart.lock with a symlink pointing to a sensitive file (like .bashrc or other startup scripts) 3. When you run helm dependency update, Helm processes the malicious Chart.yaml and writes the payload to whatever file the symlink targets 4. Code executes when the targeted file is next used (e.g., opening a new shell) Why This Works: Helm follows the symlink during the dependency update process without validating the target, allowing arbitrary file writes outside the intended chart directory.
- heisenbit 1y agoCan anyone explain in what setup an attacker who can create a symlink where Chart.lock was could not directly write .bashrc or similar? Is this related to how Git handles symlinks?
- yelirekim 1y agoHelm is a program that allows users to creates packages which other users consume. Those packages contain files that are normally generated by Helm itself, but apparently if you alter your package definition by hand you can replace Chart.lock with a symlink. As I'm typing this it's occurring to me that you probably shouldn't be able to do that. The fix they applied was to prevent the actual write from occurring when trying to write the lockfile and determining that the lockfile is a symlink. They could (should?) also validate that like, the package itself hasn't been screwed with in this manner.
- mfer 1y agoThis has nothing to do with Git. A symlink can be packaged up in a tarball and shipped from one system to another. An attacker would need to create a malicious Chart.yaml file and a Chart.lock file pointing to another file. Then ship those to a system where dependencies are then updated. This doesn't affect things like installing or upgrading a chart. Dependencies aren't updated at that time.
- quotemstr 1y agoBut I thought security vulnerabilities couldn't happen in memory-safe languages!
- qsort 1y agoBut I thought accidents wouldn't happen if we wear helmets! Clearly they're worthless!
- cluckindan 1y agoSarcasm aside: wearing a helmet causes riders to take more risks, leading to more accidents. https://www.sciencedirect.com/science/article/pii/S1369847818305941 https://www.sciencedirect.com/science/article/pii/S136984781... I’d still wear one, but also try to be more careful knowing that the helmet provides a false sense of security. I do believe the analogy holds very true with programming habits.
- cryptonym 1y agoDid you read the abstract? It says the exact opposite: > this systematic review found little to no support for the hypothesis bicycle helmet use is associated with engaging in risky behaviour.
- cluckindan 1y agoWhat! You’re lying!
- junon 1y agoThis isn't a memory bug.
- mdaniel 1y agoAnd Helm isn't written in Rust, so their snark was doubly misplaced
- codebastard 1y agoSo the attack vector is: - You have access to my file system - You have access to the helm repository You place malicious binaries outside the helm directory. Helm will now execute malicious code through the helm chart pointing outside the helm directory. Don't I have already bigger problems if you have access to my file system to place there malicious code? Is the danger here that one can get an execute permission? But if you can manipulate my helm chart why can you not also place the malicious code in the helm directory?
- deleted 1y ago[deleted]
- romaaeterna 1y ago> You place malicious binaries outside the helm directory No, helm is the one doing this part in the vuln. Chart.lock is made a symlink to some important file, and helm will happily write to it.
- Joker_vD 1y agoYeah, there is a rather strong "downloading and executing arbitrary code from the Internet may lead to execution of arbitrary code" kind of vibe there.
- steveBK123 1y agoAnd yet you just described the behavior of many mid-size company "DevOps" departments.
- captn3m0 1y agoStarting on the other side of the airtight hatchway: https://devblogs.microsoft.com/oldnewthing/20221004-00/?p=107246 https://devblogs.microsoft.com/oldnewthing/20221004-00/?p=10...
- nijave 1y agoSeems the normal mitigations apply i.e. validate with hash or save a local copy. Validate new versions before adopting
- shreeramexim655 1y ago[dead]
- mkagenius 1y agoAs an aside, all these tools like aider, claude desktop ask for shell access to run codes. Allowing LLMs to generate charts and what not via shell execution is a bad idea.
- xyst 1y agoPretty cool and nice find. I already have a "malicious" Chart.yaml in mind for this attack just based on the description of vuln. Fortunately, my dotfiles are managed with nix so trying to write to those files on a read only partition will raise many red flags for me. I don't use bash, but maybe should write a dummy .bashrc (and other start up script equivalents for fish) as some sort of canary. If I happen to overlook the malicious shell script crafted in a dependency on helm chart, I would get nasty errors that a process was trying to write to a read only file.
- ivan4th 1y agoHelm is an abomination, as the whole idea of using a text template engine to generate YAML is. And this vulnerability adds insult to injury ;) Sorry, just can't really recover from trauma of counting spaces and messing up newlines, etc. when writing Helm templates. You know, Lisp "sucks" because "you need to count parenthesis" (you actually don't), yet Helm is a widely accepted technology where you need to count spaces for (n)indent ;)
- fao_ 1y agoYeah, vi has supported % as "jump between matching parenthesis" since it's original release in the 1970s, and vim by default will do simple parenthesis matching and highlighting, I don't see why everyone is so scared of touching lisp for these reasons with modern editors (if your editor doesn't support either of the above... maybe it's not modern enough?)
- JohnMakin 1y agoThis isn't a uniquely helm thing though, they mostly use modified go templating. Lots of other things do this with yaml as well.
- deathanatos 1y ago… and I think I'd argue that the parent's argument against the tooling would apply equally as well to those "other things", too. The alternative here is something that manipulates the data structure directly. E.g., it might permit me to say: my_config_map.data["key"] = some_string_value (This is in some pseudo-imperative language, vs. the parent's Lisp, but that distinction isn't particular relevant to the core of their argument, I think.) And then at the end, the thing itself takes care of converting the resulting objects to YAML, thus preventing me from inadvertently turning what is meant to be a string into something like an accidental YAML-injection that results in terrible errors because I miscounted the number of spaces to indent something.
- JohnMakin 1y agoI wrote a small terraform wrapper around helm provider that basically does what you’re saying. official kubernetes + tf support is poor, but it’s been working well for me. I rarely if ever have to touch the yaml templates that I maintain. however, this is usually true with working with helm in general if you are using charts other people maintain. That’s one of the strengths of helm. you just shove your values into the chart and it should work. Maintaining charts is not fun though which is why I wrote the wrapper for my purposes.