review-topic-input-boundary-security

name

review-topic-input-boundary-security

description

Load when a PR touches SSRF/URL-fetch guards (`_ensure_url_input_allowed`, `_ensure_resource_fqdn_allowed`, `WHITELISTED_DESTINATIONS_FOR_URL_INPUT`/`BLACKLISTED_DESTINATIONS_FOR_URL_INPUT`), local-file image loads (`ALLOW_LOADING_IMAGES_FROM_LOCAL_FILESYSTEM`), model-cache paths from a `model_id` (`cache_path_is_within_root`, `MODEL_CACHE_DIR`), unsafe deserialization (`pickle.loads`, `torch.load`, `weights_only`, `allow_untrusted_packages`), or archive extraction (`tarfile.extractall`, `ZipFile`).

Review topic: Input-boundary security (SSRF, path traversal, unsafe deserialization, decompression)

When this applies

Trigger on CONTENT/behaviour, not just directory. Load when the diff:

• Fetches a resource from a user/client-supplied URL, or edits any of the URL guards in inference/core/utils/image_utils.py — load_image_from_url, _ensure_url_input_allowed, _ensure_resource_schema_allowed, _ensure_resource_fqdn_allowed, _concatenate_chunks_of_network_location, _ensure_location_matches_destination_whitelist/_blacklist.
• Changes URL-input policy env in inference/core/env.py — ALLOW_URL_INPUT, ALLOW_NON_HTTPS_URL_INPUT, ALLOW_URL_INPUT_WITHOUT_FQDN, WHITELISTED_DESTINATIONS_FOR_URL_INPUT, BLACKLISTED_DESTINATIONS_FOR_URL_INPUT, ALLOW_LOADING_IMAGES_FROM_LOCAL_FILESYSTEM, ALLOW_NUMPY_INPUT.
• Builds a filesystem path from a model_id, filename, or other client string — get_cache_dir, get_model_id_cache_path, slugify_model_id_to_cache_key, cache_path_is_within_root, get_cache_file_path, or any os.path.join(MODEL_CACHE_DIR, <user string>).
• Deserializes untrusted bytes — pickle.loads, torch.load, np.load, joblib.load, or toggles weights_only / allow_untrusted_packages / ALLOW_INFERENCE_MODELS_UNTRUSTED_PACKAGES.
• Extracts an archive — tarfile.extractall, zipfile.ZipFile, shutil.unpack_archive — or decodes an image/embedding whose size is attacker-controlled.

Review checklist

Severity-tag every finding. Reference the numbered Standards below.

• BLOCK — A user-supplied URL reaches requests.get (or any egress) without passing through the full load_image_from_url guard chain (_ensure_url_input_allowed → schema → FQDN → whitelist → blacklist). [S1]
• BLOCK — The URL that is validated is not the URL that is fetched: validation runs on the raw string while the request follows redirects, or requests.get(...) is given a different URL than the one whose host was checked. The prepared/normalized URL must be both validated and fetched, and redirects to unvalidated hosts must not be silently followed (GHSA-hjmm-hr52-vrp2 / PR #2501). [S1][S2]
• BLOCK — Host parsing that a \, @, userinfo, or bracketed-IPv6 authority can bypass so the allow/deny check sees a different host than requests connects to (GHSA-rv25-cq3f-x5f2 / PR #2500). [S2]
• BLOCK — pickle.loads / torch.load(weights_only=False) / np.load(allow_pickle=True) newly applied to bytes an unauthenticated client can supply, without an explicit trust gate. [S5]
• BLOCK — tarfile.extractall / ZipFile.extractall on an archive whose member paths are not confirmed to stay within the destination (Zip-Slip / ../ traversal), or with no total-size / member-count bound (decompression bomb). [S6]
• BLOCK — A model_id or filename is joined into a path and used to read/write without the cache_path_is_within_root containment check, letting ../absolute segments escape MODEL_CACHE_DIR. [S4]
• FLAG — A new client-string-derived path skips slugify_model_id_to_cache_key and relies on the raw value staying benign. [S4]
• FLAG — Weakening a URL guard default: flipping ALLOW_NON_HTTPS_URL_INPUT / ALLOW_URL_INPUT_WITHOUT_FQDN / ALLOW_LOADING_IMAGES_FROM_LOCAL_FILESYSTEM to a more-permissive default, or bypassing the whitelist/blacklist check for a "trusted" branch. [S3][S7]
• FLAG — os.path.isfile(user_value) / cv2.imread(user_value) reachable when ALLOW_LOADING_IMAGES_FROM_LOCAL_FILESYSTEM is disabled — local-FS reads must be gated by that flag on every path (PR #957). [S7]
• FLAG — torch.load left at weights_only=False for a NEW weights path that could load safetensors/weights_only=True instead, or a new allow_untrusted_packages=True call site. [S5]
• FLAG — Image/embedding decode with no upper bound on decoded dimensions/bytes where the input is client-supplied (decompression bomb via cv2.imdecode / np.load). [S6]
• NIT — A guard whose failure message leaks the internal target it tried to reach, or a swallowed decode exception that hides which guard rejected the input.

Not blocking

• Do NOT flag torch.load(weights_only=False) on the OWLv2 embeddings cache or transformers/qwen tar extraction when the bytes originate from MODEL_CACHE_DIR populated by the Roboflow weights API, not a request body — that is a first-party artifact, not attacker input. Call it out only if a new codepath lets a client supply those bytes.
• Do NOT demand a whitelist/blacklist when WHITELISTED_DESTINATIONS_FOR_URL_INPUT / BLACKLISTED_DESTINATIONS_FOR_URL_INPUT are unset — they are opt-in operator controls (None ⇒ skip), and the schema/FQDN guards still apply.
• Do NOT treat pickle.loads in load_image_from_numpy_str as a new hole — it is gated behind ALLOW_NUMPY_INPUT (default False) and is pre-existing; flag only if the gate is removed or the default flips.
• Auth / api-key / tenant-boundary concerns are owned by review-topic-auth-and-tenant-security — cross-ref, do not re-review here. review-core-infra keeps the image_utils SSRF chain as its surface instance and cross-refs this topic for the deep rules.

What to check / Standards

S1 — All user URLs run the full guard chain. In load_image_from_url, a client URL must pass _ensure_url_input_allowed() (respects ALLOW_URL_INPUT), then _ensure_resource_schema_allowed, _ensure_resource_fqdn_allowed, _ensure_location_matches_destination_whitelist, and _ensure_location_matches_destination_blacklist BEFORE any egress. Any new URL-fetching entry point must route through the same guards, not re-implement a subset. The default posture is https-only (ALLOW_NON_HTTPS_URL_INPUT=False) and FQDN-required (ALLOW_URL_INPUT_WITHOUT_FQDN=False) (PR #497 introduced the guard chain and these env controls).

S2 — Validate-then-fetch the same, normalized URL; parse the real authority. The host that the guards check must be the host requests connects to. load_image_from_url rejects a \ in the raw netloc, prepare()s the request, then validates and fetches prepared_url — do not validate the raw string and fetch something else, and do not let the parsed host diverge from the connected host via userinfo (@), backslash, or IPv6-bracket tricks (GHSA-rv25-cq3f-x5f2 / PR #2500). Host extraction uses parsed_url.hostname (bracketing IPv6) fed to tldextract, not raw netloc. Redirects must not smuggle the request to an internal target after a benign first host passes validation (GHSA-hjmm-hr52-vrp2 / PR #2501).

S3 — Whitelist/blacklist are exact-destination allow/deny lists. _ensure_location_matches_destination_whitelist rejects unless the concatenated network location is in WHITELISTED_DESTINATIONS_FOR_URL_INPUT; _ensure_location_matches_destination_blacklist rejects if it is in BLACKLISTED_DESTINATIONS_FOR_URL_INPUT. Both are None-means-skip. The destination is _concatenate_chunks_of_network_location(...) over the tldextract result (subdomain.domain.suffix, IPv6 brackets stripped) — do not compare against raw user text or a substring, which would let evil.com/../roboflow.com or a subdomain trick slip through.

S4 — model_id / filename paths must stay inside MODEL_CACHE_DIR. get_model_id_cache_path only trusts the raw model_id as a path segment when cache_path_is_within_root (an os.path.commonpath containment check against the abspath of cache_dir_root) AND path_fits_os_limits both pass; otherwise it falls back to slugify_model_id_to_cache_key (regex to [A-Za-z0-9_-], collapse, truncate, blake2s suffix). Any new path built from a client string must apply the same containment check or slugify — never os.path.join(MODEL_CACHE_DIR, raw_model_id) and read/write directly, which a ../ or absolute model_id would escape.

S5 — Deserialize untrusted bytes safely. Never pickle.loads, torch.load(weights_only=False), or np.load(allow_pickle=True) on bytes a client can supply without an explicit trust gate. Prefer weights_only=True (as doctr_model.py and perception_encoder/pe.py do) and safetensors. allow_untrusted_packages must stay bound to ALLOW_INFERENCE_MODELS_UNTRUSTED_PACKAGES (env, default False) — a literal True at a call site is a finding. The one client-facing pickle (load_image_from_numpy_str) stays behind ALLOW_NUMPY_INPUT (default False); keep it that way.

S6 — Bound decompression and extraction. Archive extraction (tarfile.extractall, ZipFile) must confirm every member resolves within the destination (no ../, no absolute, no symlink escape) and bound total uncompressed size / member count. Image and embedding decodes (cv2.imdecode, np.load(BytesIO(...))) on client bytes must have an upper bound on decoded dimensions/bytes so a small payload cannot expand into an OOM. Only exempt archives sourced from the first-party weights cache, not request bodies.

S7 — Do not weaken input-boundary defaults. ALLOW_URL_INPUT=True but ALLOW_NON_HTTPS_URL_INPUT=False, ALLOW_URL_INPUT_WITHOUT_FQDN=False, ALLOW_NUMPY_INPUT=False are the safe defaults; ALLOW_LOADING_IMAGES_FROM_LOCAL_FILESYSTEM gates every local-FS read (both load_image_with_known_type for ImageType.FILE and the inferred os.path.isfile branch — PR #957). Flipping any of these to more-permissive defaults, or adding a "trusted"/internal bypass around a guard, needs explicit justification and is at least a FLAG.

Key files & reference PRs

• inference/core/utils/image_utils.py — SSRF guard chain: load_image_from_url (validate-then-fetch on the prepared URL), _ensure_url_input_allowed, _ensure_resource_schema_allowed, _ensure_resource_fqdn_allowed, _concatenate_chunks_of_network_location, _ensure_location_matches_destination_whitelist/_blacklist; local-FS gating in load_image_with_known_type / load_image_with_inferred_type; the ALLOW_NUMPY_INPUT-gated pickle.loads in load_image_from_numpy_str.
• inference/core/env.py — URL/input policy: ALLOW_URL_INPUT, ALLOW_NON_HTTPS_URL_INPUT, ALLOW_URL_INPUT_WITHOUT_FQDN, WHITELISTED_DESTINATIONS_FOR_URL_INPUT, BLACKLISTED_DESTINATIONS_FOR_URL_INPUT, ALLOW_LOADING_IMAGES_FROM_LOCAL_FILESYSTEM, ALLOW_NUMPY_INPUT, ALLOW_INFERENCE_MODELS_UNTRUSTED_PACKAGES, MODEL_CACHE_DIR.
• inference/core/cache/model_artifacts.py — path-traversal containment: get_cache_dir, get_model_id_cache_path, cache_path_is_within_root, slugify_model_id_to_cache_key, path_fits_os_limits, get_cache_file_path.
• inference/core/models/inference_models_adapters.py — allow_untrusted_packages=ALLOW_INFERENCE_MODELS_UNTRUSTED_PACKAGES call sites (the pattern every model adapter must follow).
• inference/models/** — torch.load sites: weights_only=True in doctr/doctr_model.py, perception_encoder/vision_encoder/pe.py; weights_only=False on the first-party OWLv2 embeddings cache in owlv2/owlv2.py; tarfile.extractall in transformers/transformers.py, qwen25vl/qwen25vl.py, qwen3vl/qwen3vl.py; np.load(BytesIO(...)) embedding/mask decode in sam/.
• Reference PRs / advisories: #2500 (GHSA-rv25-cq3f-x5f2 — backslash/userinfo authority-parsing allowlist bypass), #2501 (GHSA-hjmm-hr52-vrp2 — validate-first-FQDN-then-follow-redirect-to-internal-target), #497 (original SSRF guard chain + URL-input env controls), #957 (gate local-filesystem image reads behind ALLOW_LOADING_IMAGES_FROM_LOCAL_FILESYSTEM, stop passing user value to os.path.isfile).