Harden XTCE schema validation against local file read (CWE-73) and SSRF (CWE-918) - #267
Harden XTCE schema validation against local file read (CWE-73) and SSRF (CWE-918)#267blakedehaas wants to merge 13 commits into
Conversation
blakedehaas
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #267 +/- ##
==========================================
- Coverage 94.92% 94.49% -0.43%
==========================================
Files 48 49 +1
Lines 3904 4163 +259
==========================================
+ Hits 3706 3934 +228
- Misses 198 229 +31 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
158b3d6 to
eb0ed24
Compare
Treat a document-supplied xsi:schemaLocation as untrusted: reject local filesystem paths (CWE-73) and restrict schema URLs to https on an allowlisted, non-internal host (CWE-918). Bundle the OMG XTCE 1.2 schema so the standard case validates offline with no network request. - Bundle SpaceSystem.xsd; resolve it offline by URL before any download. - Add allowed_schema_hosts / allow_insecure_http / allow_schema_download to validate_xtce and the spp validate CLI, with SPP_ALLOWED_SCHEMA_HOSTS and SPP_ALLOW_INSECURE_HTTP env overrides; export DEFAULT_ALLOWED_SCHEMA_HOSTS. - Block internal/link-local IP-literal hosts (169.254.169.254, 127.0.0.1). - Restore trusted local_xsd: absolute paths work from any cwd again. - Cache only content that validates as XSD; cap download size; 0600 perms. - Use accurate error codes (DISALLOWED_SCHEMA_LOCATION) and surface messages. - Move mock_schema_download fixture to conftest; add security regression tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
medley56
left a comment
medley56
left a comment
There was a problem hiding this comment.
🤖 This review was generated with Claude and reviewed by Gavin (@medley56).
Hi @blakedehaas — thanks for kicking this off. Getting the SSRF/LFI hardening started was the hard part, and the structure you set up (a dedicated schema-load path, the caching layer, the test scaffolding) is what we built the final version on top of. As code owner I took the branch the rest of the way, and I want to walk you through what we changed and why so nothing here is a surprise.
The core reframe: a document's xsi:schemaLocation is fully attacker-controlled whenever we validate an untrusted document, so the fix has to treat it as untrusted at every step — while keeping the operator-supplied local_xsd fully trusted. The original approach guarded some of the surface but left the highest-severity path open, so we re-centered everything on that trust boundary.
What was still exploitable (and is now closed):
- SSRF (the 7.4 High) was not actually mitigated. The scheme check allowed any
http/httpsURL, so the advisory's own PoC —http://169.254.169.254/latest/meta-data/...— sailed through. We replaced the scheme-only check with https-by-default + a host/URL allowlist + an internal-address guard, so metadata/loopback/private targets are blocked outright. - LFI was only partly closed.
startswith("/")missed Windows/UNC paths, and a sibling-directory string-prefix bug let some traversals through. Document-derived local paths are now rejected wholesale (any non-http(s)location) — a local schema must come throughlocal_xsd.
What we added:
- Bundled the OMG XTCE 1.2 schema, so the standard case validates offline with no network request — this removes the SSRF surface for the common path entirely and makes validation robust to OMG being unreachable.
- Configurable, safe-by-default controls:
allowed_schema_hosts(hosts or exact URLs),allow_insecure_http(dangerous opt-in), andallow_schema_download, all mirrored on the CLI and with env-var overrides. Default download stays on but allowlisted.
What we fixed from the first iteration: restored absolute local_xsd (it was being silently rewritten to a bare filename), stopped caching un-validated bytes, added a download size cap, gave rejections accurate/surfaced error codes, and moved the mock_schema_download fixture into conftest.py (the integration test couldn't find it, so it was erroring at setup).
Full rationale is in the changelog and docs/source/users.md; details are in the inline comments. Happy to hop on a call if any of the decisions are worth talking through — especially the "download on-by-default but allowlisted" choice, since that was the main judgment call.
| # covers absolute POSIX/Windows/UNC paths, relative paths, bare filenames, and non-http schemes | ||
| # such as file:// and ftp://. Host/scheme allowlist policy is enforced later, at load time, | ||
| # after bundled-schema resolution. | ||
| if urlparse(schema_location).scheme not in ("http", "https"): |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
LFI fix (CWE-73). The original guard here was schema_location.startswith("/"), which only catches POSIX absolute paths — it missed Windows drive paths (C:\...), UNC paths (\\server\share), and left relative traversals (../../etc/passwd) to a downstream check that had a sibling-directory prefix bug. We inverted the logic: anything that isn't an http(s) URL is treated as a local reference and rejected here, up front. A local schema now has exactly one path in — the trusted local_xsd argument. Regression-tested against /etc/hostname, ../../../etc/passwd, C:\..., UNC, and file:// in test_validation_security.py.
| return False | ||
|
|
||
|
|
||
| def _enforce_schema_url_policy(schema_location: str, allowed_hosts: frozenset[str], allow_insecure_http: bool) -> None: |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
SSRF fix (CWE-918) — the headline change. The previous version allowed any http/https URL, which means the advisory's PoC (http://169.254.169.254/latest/meta-data/iam/security-credentials/) was still fully reachable — a scheme check is not an SSRF control. This helper now enforces three things together: https only (unless allow_insecure_http is explicitly set), the host/URL allowlist (default www.omg.org; entries can be bare hosts or exact URLs, matched exactly — never a suffix test), and the internal-address guard below. All three are independent, so relaxing one (e.g. enabling http for an internal mirror) doesn't reopen the others.
| return _BUNDLED_SCHEMA_DIR / filename | ||
|
|
||
|
|
||
| def _reject_internal_host(host: str | None, schema_location: str) -> None: |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
This is the guard that deterministically kills the cloud-metadata and loopback PoCs. IP literals in the URL (169.254.169.254, 127.0.0.1, 10.x, etc.) are rejected as private/loopback/link-local/reserved regardless of the allowlist. We scoped it to IP literals on purpose: hostnames are governed by the allowlist, and doing DNS resolution here would add a network round-trip plus a TOCTOU/rebinding wrinkle. The residual DNS-rebinding case (an allowlisted hostname that resolves internally) is noted in the docstring as a documented, much narrower follow-up.
| """ | ||
| location = str(schema_location) | ||
|
|
||
| # 1. Bundled schema (offline, no policy needed — it is our own file). |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
Bundling (offline by default). Resolution order is now: (1) bundled schema → (2) trusted local_xsd → (3) allowlisted download. Because the OMG XTCE 1.2 schema ships in the package (xtce/schemas/), a document referencing the standard www.omg.org/.../SpaceSystem.xsd URL validates from disk with zero network calls — verified by a test that patches urlopen and asserts it's never called. This both removes the SSRF surface for the common case and makes validation resilient to OMG downtime. Only XTCE 1.2 is bundled today; a 1.1 document falls through to the allowlisted-download path (OMG-hosted, so it still works). Adding 1.1 offline is a one-line _BUNDLED_SCHEMAS entry.
| # local_xsd is trusted, caller-supplied input: open it directly, at whatever path | ||
| # (absolute or relative) the caller provided. No confinement is applied because this | ||
| # is not the attacker-controlled surface (unlike document-derived xsi:schemaLocation). | ||
| schema_location = str(local_xsd) |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
Trust boundary + regression fix. The prior version treated local_xsd like untrusted input: an absolute path was rewritten via relative_to(cwd()), falling back to just the basename when that failed — which silently broke absolute --local-xsd from any other working directory, or worse, validated against a same-named file that happened to sit in cwd. local_xsd is operator-supplied and trusted, so it's now opened directly at whatever path was given. The confinement effort belongs on the untrusted document path (above), not here. Restored behavior is covered by test_validate_xtce_absolute_local_xsd_is_used_directly.
| ) | ||
|
|
||
| schema, version = _parse_schema_content(schema_content, schema_location) | ||
| # Only cache content that validated as an XSD. |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
Cache hardening. Previously the raw HTTP response was written to ~/.cache/... before it was parsed, so a non-schema body (an SSRF probe response, an error page, exfiltrated data) would persist on disk — which the advisory explicitly called out. Now we download with a size cap (MAX_SCHEMA_BYTES), validate the bytes as an XSD, and only then write to cache. We deliberately cache the raw validated bytes (not the post-_fix_known_schema_issues version) so the download-avoidance benefit is preserved without baking a fixup into the cache — a future improvement to the fix logic still applies on the next load. Cache files are also written 0600.
| self.validation_result = validation_result | ||
| # Optional machine-readable code so callers (e.g. _validate_xtce_schema) can map | ||
| # the failure onto the correct ValidationResult error_code instead of a generic one. | ||
| self.error_code = error_code |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
Small but worth flagging: in the first iteration the "Path traversal detected" error was raised inside a try whose own except Exception immediately re-wrapped it as a generic "Invalid schema path", so the real reason never reached the caller — and every rejection from _find_schema_url was reported as MISSING_SCHEMA_LOCATION even when a location was present but disallowed. Errors now carry an explicit error_code, so a disallowed/blocked location surfaces as DISALLOWED_SCHEMA_LOCATION with its actual message, distinct from a genuinely missing one.
| "ftp://example.com/schema.xsd", # non-http scheme | ||
| ], | ||
| ) | ||
| def test_document_url_ssrf_is_rejected(location): |
There was a problem hiding this comment.
🤖 This comment was generated with Claude and reviewed by Gavin (@medley56).
New regression suite anchored directly to the advisories: every PoC (metadata IP, loopback, arbitrary external host, absolute/relative/Windows/UNC/file:// local paths) asserts rejection and that urlopen is never called; plus positive coverage for the allowlist (arg, env var, exact-URL entries), the allow_insecure_http opt-in still being blocked by the internal-IP guard, offline bundle use, and the cache behavior. Separately, the mock_schema_download fixture moved to tests/conftest.py — it lived in this module before, so tests/integration/test_cli.py::test_validate_xtce couldn't resolve it and was erroring at setup rather than running.
medley56
changed the title
2 participants
Summary
Hardens XTCE schema validation against two reported vulnerabilities in
validate_xtce. Both share one root cause: a document'sxsi:schemaLocationis attacker-controlled but was used to read files and make network requests without restriction. Fixes #266.Problems being fixed
xsi:schemaLocationat any URL, causing the validating host to issue outbound requests — e.g. the cloud metadata endpointhttp://169.254.169.254/latest/meta-data/iam/security-credentials/for IAM credential theft — and the response was cached to disk, persisting exfiltrated data./etc/hostname,../../etc/passwd), causing arbitrary local files to be read as a "schema".How we fix it
The document-supplied
xsi:schemaLocationis now treated as untrusted, while an operator-suppliedlocal_xsdremains trusted. Schema resolution is:local_xsd(trusted). Opened directly at any path.httpson an allowlisted host (defaultwww.omg.org). Internal/link-local targets (169.254.169.254,127.0.0.1, private ranges) are always blocked. Local filesystem paths from a document are rejected outright — uselocal_xsd.Additional hardening:
allowed_schema_hosts(hosts or exact URLs),allow_insecure_http(dangerous opt-in; host allowlist + internal-address guard still apply),allow_schema_download. Default download stays on but allowlisted.DEFAULT_ALLOWED_SCHEMA_HOSTSis exported.Behavior changes
xsi:schemaLocationis a local path or a non-allowlisted / non-https URL is now rejected — passlocal_xsd, extend the allowlist, or (for http) opt in explicitly.local_xsd/--local-xsdagain works from any working directory (fixes a regression that rewrote it to a bare filename).Testing
New
tests/unit/test_xtce/test_validation_security.pyasserts every advisory PoC is rejected with no outbound request, plus positive coverage for the allowlist (arg/env/exact-URL), theallow_insecure_httpopt-in still blocking internal IPs, offline bundle validation, and cache behavior. Full suite passes; docs and changelog updated.PR description drafted with Claude and reviewed by Gavin (@medley56).