Fixes a bypassable start_with? prefix guard introduced in the #1198 arbitrary-file-read fix. The guard operated on raw string paths, so ../ segments after an allowed prefix (e.g. /app/public/../../../etc/passwd ) passed the check while File.open resolved them to arbitrary locations. A second bypass existed in cama_tmp_upload 's HTTP branch, where a URL containing the site URL as a substring was converted to a local filesystem path without canonicalization.
This PR fixes both bypasses and hardens UserUrlValidator . Following a full self-review of the changes, it also fixes a set of correctness/robustness issues found in the new code, extracts the duplicated upload-path logic into a shared module, and bumps two transitive sanitizer dependencies.
4 sink guards: Replaced the raw start_with? prefix check with File.expand_path canonicalization + prefix validation ( expanded == root || expanded.start_with?(root + File::SEPARATOR) ) in both upload_file and cama_tmp_upload , in both RuntimeUploaderConcern and UploaderHelper (defense-in-depth — each guard independently self-sufficient, since plugins call these as public API).
Host comparison: Replaced substring-based site-URL detection in cama_tmp_upload 's HTTP branch with proper host + port comparison, preventing URL-to-path conversion bypasses (e.g. ).
Improved UserUrlValidator : Added resolved_ip (for DNS pinning), reject_path_traversal: (detects ../ and URL-encoded %2e%2e traversal in URL paths), resolve: false mode (static-only IP validation without DNS, for custom-scheme URLs), a validate_external_https class method, and an env-var-controlled skip ( CAMALEON_SKIP_URL_VALIDATION ). Stronger IP validation via Resolv::IPv4/IPv6::Regex and out-of-range octet detection.
data: URI fix: Skip UserUrlValidator for data: URIs in the crop_url action, resolving the data-URI upload regression from Security: Fix arbitrary server-side file read via media upload #1198 (the validator blocked them because DNS resolution fails on a schemeless host).
i18n: Added https_only_url and path_traversal keys under the existing camaleon_cms.admin.validate.* namespace.
Shared module (refactor)
Extracted the canonicalization guard, same_site_url? , site_url_path , and a new path_within? helper into CamaleonCms::UploaderPathSecurity , included by both RuntimeUploaderConcern and UploaderHelper , so the four sink guards and the same-site logic can no longer drift between the two copies.
Hardening from self-review
A max-effort review of the diff surfaced the following, each fixed in its own commit with tests:
(security) data: write traversal — the re-enabled data: branch wrote the base64 payload to File.join(tmp_path, params[:name]) with only the basename sanitized by uploader_verify_name , so ../ in name escaped tmp_path before the canonicalization guard ran. Now the name is reduced to File.basename and a path_within?(tmp_path) guard confirms the write stays inside tmp_path .
Path-traversal false positives — reject_path_traversal compared uri.path to uri.normalized_path , so Addressable's encoding normalization flagged legitimate URLs with percent-encoded unreserved characters ( %7E → ~ , %2D → - ) or a /./ segment as traversal. Now decodes the path and checks for actual .. path segments (still catches %2e%2e ).
Same-site URL parser — same_site_url? / site_url_path used stdlib URI.parse , which raises on paths containing spaces or non-ASCII characters, misclassifying legitimate same-site URLs as remote. Switched to Addressable, comparing host case-insensitively and by inferred port.
Mount subpath — site_url_path now strips the site's mount subpath ( relative_url_root ) before the locale prefix, so same-site media on subpath-mounted installs maps to the correct on-disk path.
Locale stripping — only strip a leading locale segment when the stripped path points at an existing file under the public dir, to avoid dropping a real first directory that merely shares a language code's name.
Static limited-broadcast — use the already-computed native IP so the IPv4-mapped form [::ffff:255.255.255.255] is detected in resolve: false mode.
Trailing-dot FQDN — don't append a second dot to a hostname that already ends in one (e.g. example.com. ), which previously failed DNS resolution.
Shorthand decimal IPv4 — reject an all-numeric dotted hostname with an out-of-range octet regardless of octet count (e.g. 192.168.257 ), closing an SSRF gap in resolve: false mode.
Test DNS — stub DNS in the new reject_path_traversal specs so they don't perform live lookups on network-isolated CI.
Cleanup — removed the unused RETURN_URL_OPTIONS constant.
crop_url nil folder — a successful crop_url calls upload with no :folder , so upload_file received folder: nil and crashed at valid_folder_path?(nil) (and would also break File.join ). Coerce a nil folder to '' (root) in the shared upload_file , matching the existing formats coercion.
Dependency security bumps
loofah 2.25.1 → 2.25.2
rails-html-sanitizer 1.7.0 → 1.7.1
bin/rubocop clean across all changed Ruby files.
bin/rspec green across the validator, uploader-helper, and media-controller request specs (121 examples, 0 failures). New regression tests cover items Does it work with PostgreSQL? #1 – Fixes PostgreSQL name in README #8 , the crop_url nil-folder fix, and the same-site / site_url_path conversions.
Path-traversal bypass reported by Jose Rivas (Zero Trust Offsec).
The full story
This article is one source in a clustered incident — the cluster page carries the summary, timeline and every other outlet covering it.
