review-topic-input-boundary-security

# 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`).