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

Đang mở
  • Có gì không ổn?
#747 0 bình luận 0 reaction 1 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@mattmillerai đang làm issue này rồi.

Từ ngày 21/8/2026.

Đánh giá

Issue này chưa được đánh giá.

Mô tả

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.

Ngôn ngữ chính
Python
Star
1k
Fork
176
Merge trung bình
1 ngày 11 giờ
Pull request đã merge (30 ngày)
87

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của Comfy-Org/comfy-cli

Tất cả issue của Comfy-Org/comfy-cli

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.