T09 · Insecure Skill Coding Practices
- Location
- scripts/downloader.py:430
- Finding
- Plaintext HTTP and HTTPS-to-HTTP Redirect Downgrades Permit Evidence Tampering<![CDATA[ ## Vulnerability Details **File Location**: `scripts/downloader.py:430-434`, `scripts/downloader.py:574-594`; reachable through `scripts/adapters/qr.py:204-219` **Vulnerability Type**: Insecure transport and redirect downgrade **Risk Level**: Medium ### Vulnerable Code The downloader explicitly accepts both encrypted HTTPS and plaintext HTTP: ```python if parsed.scheme.lower() not in {"http", "https"}: raise DownloadSecurityError("Only HTTP and HTTPS URLs are allowed") if not parsed.netloc or hostname is None: raise DownloadSecurityError("URL must include a host") if parsed.username is not None or parsed.password is not None: raise DownloadSecurityError("URL userinfo is not allowed") ``` Redirect destinations are joined and revalidated for public addressing, but the code does not preserve the original transport security level or reject an HTTPS-to-HTTP downgrade: ```python if response.status in {301, 302, 303, 307, 308}: if redirects_followed >= 5: raise DownloadRedirectError("Redirect limit exceeded") location = response.getheader("Location") if not location: raise DownloadRedirectError("Redirect response has no Location") current_url = urljoin(current_url, location) redirect_chain.append(current_url) redirects_followed += 1 continue ``` Host-decoded QR URLs flow directly into this downloader: ```python def resolve_qr_payload( payload: object, workspace: str | Path, *, qr_image_source_id: str, max_bytes: int, timeout: float, ) -> QrResolution: original_url, source_id = _decoded_url(payload, qr_image_source_id) validate_public_url(original_url) result = download_public_file( original_url, workspace, max_bytes=max_bytes, timeout=timeout, ) destination = _validate_result(result, workspace) return QrResolution._from_download(source_id, original_url, result, destination.name) ``` ### Technical Analysis The downlo ...[truncated 2423 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Require HTTPS by default in `_validated_target()`: ```python if parsed.scheme.lower() != "https": raise DownloadSecurityError("Only HTTPS URLs are allowed") ``` 2. Enforce redirect transport continuity before following each redirect: ```python next_url = urljoin(current_url, location) current_scheme = urlsplit(current_url).scheme.lower() next_scheme = urlsplit(next_url).scheme.lower() if current_scheme == "https" and next_scheme != "https": raise DownloadRedirectError("HTTPS-to-HTTP redirect is not allowed") ``` 3. Apply the same HTTPS requirement in `scripts/adapters/qr.py` so unsafe QR payloads fail before network access. 4. If a legacy authority is available only through HTTP, use a narrowly scoped, explicit source-policy exception rather than globally permitting HTTP. Such an exception should require: - A preconfigured hostname allowlist. - An expected cryptographic digest or authenticated detached signature. - Independent corroboration from an HTTPS source. - Clear provenance indicating that transport authentication was unavailable. - No transmission of tokens, personal information, or sensitive query parameters. 5. Add regression tests covering: - Direct HTTP rejection. - HTTPS-to-HTTP redirect rejection. - HTTP redirect to private, loopback, or metadata addresses. - HTTPS-to-HTTPS redirects remaining functional. - QR payloads containing HTTP URLs being rejected. - Legacy exceptions failing closed when a digest or corroborating source is absent. 6. Preserve the existing DNS pinning, connected-peer verification, response-size limits, media-type restrictions, and atomic workspace storage; these are useful controls but should complement, not replace, authenticated transport. ]]>
