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

Abierto
  • ¿Algo no está bien?
#747 0 comentarios 0 reacciones 1 asignado Ver en GitHub

Los mantenedores suelen responder en 1 día

@mattmillerai ya está trabajando en esto.

Desde el 21/8/2026.

Evaluación

Este issue todavía no se ha evaluado.

Descripción

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.

Lenguaje dominante
Python
Estrellas
1k
Forks
176
Merge medio
1 d 11 h
PR fusionados (30 d)
87

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de Comfy-Org/comfy-cli

Todos los issues de Comfy-Org/comfy-cli

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.