Skip to content

Plugin install: unvalidated plugin.yaml name escapes HELM_PLUGINS (arbitrary file write) #825

Description

@alexmond

HelmPluginInstaller builds the install destination from the name field of the
plugin's own plugin.yaml, without validating it:

String name = resolveName(manifest, pluginRoot);   // manifest.getName(), verbatim
Path dest = this.paths.pluginsDir().resolve(name); // no normalize, no containment check
Files.createDirectories(dest.getParent());
FileUtils.copyDirectory(pluginRoot.toFile(), dest.toFile());

resolveName returns the manifest value as-is, and there is no validation of plugin
names anywhere in the app.plugin package. A name containing path separators or
.. segments therefore escapes HELM_PLUGINS and writes the plugin tree to an
arbitrary location the process can write to.

Confirmed with a probe against the real installer — a manifest whose name walks up
one directory installs outside the plugins dir and leaves its files there:

installed-to = <pluginsDir>/../<other>/OUTSIDE/pwned
escaped      = true
outside-tree = <other>/OUTSIDE/pwned/plugin.yaml

Why this is more than "installing a plugin is already trusting it"

copyDirectory happens before runInstallHook, so the write is not gated by the
exec policy. In READ_ONLY mode HelmPluginExecGuard blocks the install hook — the
mode a user would rely on to inspect an untrusted plugin without running it — but the
out-of-tree file write still occurs. The traversal is reachable in exactly the
configuration meant to be safe.

Upstream

Same class as Helm's GHSA-vmx8-mqv2-9gmg (high, 2026-04-09), "Path traversal in plugin
metadata version enables arbitrary file write outside Helm plugin directory". Upstream's
sink was the version field; ours is name.

Suggested fix

The correct guard already exists in this codebase — RepoManager.validateRepoName
rejects null/blank/./.. and anything outside [A-Za-z0-9._-]+. Apply the same
validation to the resolved plugin name (both the manifest value and the
directory-name fallback), and additionally assert the resolved dest is contained by
pluginsDir() as defence in depth, matching the containment check the tar extraction
path already performs at HelmPluginInstaller:247-249.

Not affected

Archive extraction is already guarded in both places and does not need changing:

  • HelmPluginInstaller:247-249normalize() + startsWith(dest)
  • RepoManager:1292-1294 — canonical-path containment

So the chart-extraction advisory (GHSA-hr2v-4r36-88hr) does not apply here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions