←back to thread

Helm local code execution via a malicious chart

(github.com)
171 points irke882 | 3 comments | 09 Jul 25 05:49 UTC | HN request time: 0.52s | source
Show context
TheDong ◴[09 Jul 25 06:54 UTC] No.44507023[source]▶
That 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/76fdba4c8c2a4829a6b7abb4...), wouldn't the fix be instead that "A chart cannot contain any symlinks outside of its root"?

replies(1): >>44507361 #
yelirekim ◴[09 Jul 25 07:59 UTC] No.44507361[source]▶
I 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

replies(1): >>44507519 #
1. TheDong ◴[09 Jul 25 08:25 UTC] No.44507519[source]▶
What 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.

replies(2): >>44507570 #>>44509014 #
2. yelirekim ◴[09 Jul 25 08:31 UTC] No.44507570[source]▶
Ya, 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...

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.

3. nijave ◴[09 Jul 25 12:08 UTC] No.44509014[source]▶
I 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