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

未关闭
  • 哪里不对？
#747 0 条评论 0 个 reaction 已指派 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 小时
30 天内合并 PR
87

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

Comfy-Org/comfy-cli 的其他 Issue

查看 Comfy-Org/comfy-cli 的全部 Issue

相似的 Issue

更多 Python Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。