`registry-install`: registry-supplied `version` string is joined unvalidated into the download path

Aperta
  • Qualcosa non va?
#747 0 commenti 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@mattmillerai ci sta già lavorando.

Dal 21/8/2026.

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

bug

What — comfy node registry-install builds the archive's download destination out of the registry-supplied version string, with no validation:

comfy_cli/command/custom_nodes/command.py:1410

local_filename = node_specific_path / f"{node_id}-{node_version.version}.zip"
...
download_file(node_version.download_url, local_filename)   # :1413

node_version.version is api_node_version["version"] taken verbatim from the registry response (comfy_cli/registry/api.py:259) — mandatory field, no validation anywhere between the HTTP response and the path join. pathlib.__truediv__ does not sanitize, and download_file does local_filepath.parent.mkdir(parents=True, exist_ok=True) (comfy_cli/file_utils.py:837) before writing, so it will happily create the traversed directory chain first.

A version of 1.0.0/../../../../some/other/dir/x therefore writes attacker-supplied bytes (the body of downloadUrl) outside custom_nodes/.

Scope, stated honestly — the last path component is always …{version}.zip, so the written file's name still ends in .zip. This is an arbitrary-directory write of a .zip-suffixed file with fully attacker-chosen content, not an arbitrary-filename overwrite. That rules out the obvious "clobber a dotfile" escalation, and is why I'd call this moderate rather than high. It still lets a publisher plant content anywhere the user can write, overwrite an existing .zip, and fill a disk. The cleanup os.remove(local_filename) at :1428 only runs when extraction returns normally.

For the avoidance of doubt about the other two values in that line: node_id is the user's own CLI argument (registry_install(node_id: str, …) at :1341), not registry-supplied, and node_specific_path derives from it — so this is specifically about version. And the extraction itself is fine: extract_package_as_zip uses zipfile.extractall, which sanitizes member names and never materializes symlinks (I tested ../.., an absolute member, and an S_IFLNK member on CPython 3.12.3 — all three land inside the destination, the symlink as a regular file). The bug is where the archive file itself is written, not where it is unpacked.

Attacker position — the registry API (api.comfy.org), or more plausibly a node publisher who can put an arbitrary string in project.version in their pyproject.toml at publish time (comfy_cli/registry/api.py:121 sends it). Whether that is exploitable end-to-end depends on whether the registry backend enforces semver on version, which I could not check from the CLI repo — so treat this as "the client should not be relying on that", not as a demonstrated live exploit.

The fix already exists in this repo. comfy_cli/command/models/models.py:130-157 _reject_unsafe_component does exactly this check, and its docstring describes exactly this situation:

local_filename and basemodel are joined into the destination path, and both can come straight off the CivitAI API response … which is remote input, accepted without a prompt in non-interactive runs. pathlib/os.path.join do not sanitize .. or an absolute component, so an unvalidated value writes outside the workspace.

It rejects ., .., absolute paths, drive letters, / and \. Applying it to node_version.version (and cheaply to node_id) before command.py:1410 closes this with no new code.

Assignee note — git log -S puts the original line at 6b4786c (James Kwon, 2024-05-11, "Implementation for installing node from registry (#41)"), but his last commit to this repo was 2024-07-22. Assigning to @mattmillerai instead, who wrote _reject_unsafe_component — the exact guard this path is missing — and is the current active owner of both custom_nodes/command.py and registry/api.py. Reassign if that's wrong.

Found during a cross-repo security review of comfy-cli, 2026-08-21. Related: #746, #725.

Lingua principale
Python
Stelle
1k
Fork
176
Merge medio
1g 11h
PR unite (30g)
87

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Comfy-Org/comfy-cli

Tutte le issue di Comfy-Org/comfy-cli

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.