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

オープン
  • 何か問題がありますか？
#747 コメント 0 件 リアクション 0 件 担当者 1 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@mattmillerai がすでに取り組んでいます。

2026年8月21日 から。

評価

この issue はまだ評価されていません。

説明

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.

主要言語
Python
スター
1k
フォーク
176
平均マージ
1日 11時間
マージ済み PR（30日）
87

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

Comfy-Org/comfy-cli のほかの issue

Comfy-Org/comfy-cli の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。