Skip to content

Download-queue SSRF follow-ups (from #9492 review) #9493

Description

@lstein

Follow-up items surfaced during the adversarial review of #9492 (download-queue SSRF + dest confinement). None are blockers for that PR; they are collected here so the PR can merge and these can be addressed separately.

1. _api_job relativizes dest but not download_path

download_queue.py::_api_job rewrites job.dest to a path relative to the download-queue directory for API responses, but leaves download_path untouched. Once a download starts, download_path is populated as an absolute server path (e.g. /home/.../download_cache.downloads/...) and returned verbatim, leaking the cache's absolute location. Admin-only after this change, so cosmetic — but inconsistent with the stated goal of keeping the API destination relative. Fix: relativize download_path the same way (when it is under the queue root), or document that it is intentionally absolute.

2. download_proxy bypasses the in-process socket guard

When download_proxy is configured, requests routes proxied connections through HTTPAdapter.proxy_manager_for(), whose pools never receive the guarded ConnectionCls set in SsrfGuardedAdapter.init_poolmanager. The socket-level peer check therefore does not run for proxied requests (confirmed empirically: a request through a loopback proxy returned 200 rather than being blocked). This is documented in the PR ("the proxy must enforce the public-address policy") and is a defensible punt, but it is a sharp edge: enabling download_proxy silently disables the in-process guard. Options: log this explicitly at startup when a proxy is set (beyond the current generic proxy warning), and/or override proxy_manager_for() to install the guarded pool classes on proxy managers too.

3. Router error message is imprecise for non-address failures

In download_queue.py::download, the except UnsafeDownloadURLException handler always returns "Download URL resolves to a non-public address.", even when the exception was actually raised for an unsupported scheme, missing host, or invalid port. These variants are currently unreachable because source is typed AnyHttpUrl, so this is cosmetic — but the message should reflect the actual failure if the typing ever changes.

4. (Pre-existing, out of scope for #9492) alibabacloud._download_image unguarded fetch

invokeai/app/services/external_generation/providers/alibabacloud.py::_download_image fetches a provider-response URL with no address guard. A malicious or compromised provider response could point it at an internal address. Should use the guarded session from invokeai/app/util/ssrf.py.

5. (Pre-existing, out of scope for #9492) custom_nodes.py passes source straight to git clone

custom_nodes.py hands the user-supplied source directly to git clone without a scheme check. Node installation is admin-only and RCE-by-design, so this is low priority, but ext:: and other non-http(s)/git transports are worth rejecting.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions