Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo

### Security

- `$isSafeRedirectUrl()` now normalizes input per WHATWG URL parsing before classification — embedded ASCII tab/CR/LF are stripped and leading/trailing ASCII whitespace is trimmed — and any URL retaining a C0 control character (`Chr(0)`-`Chr(31)`, `Chr(127)`) is rejected outright. Browsers apply this normalization before navigation, so a URL like `<TAB>//evil.com` or `/<LF>/evil.com` was classified as a same-origin relative path by the pre-strip gate but navigated to `//evil.com` once the browser stripped the whitespace, smuggling an open redirect past `redirectTo()`'s default `allowExternalRedirects=false` policy. Mirrors `#2898`'s strip-then-classify shape and runs ahead of the backslash check so post-strip backslashes are still caught. Deferred-scope finding from `#2898`; specs mirror the prior-art Chr(9)/Chr(10)/Chr(13) test-vector style
- `$isSafeRedirectUrl()` rejects backslash-containing URLs (`/\evil.com`, `\/evil.com`, `\\evil.com`) and schemeless-authority URLs (`https:/evil.com`, `javascript:alert(1)`) instead of returning them as safe. Browsers normalize backslashes to forward slashes and single-slash schemes to authority form, so any of those vectors smuggled past the previous check would navigate off-site after `redirectTo()`. Scheme detection now uses the RFC 3986 grammar (`ReFindNoCase("^[a-z][a-z0-9+.-]*:")`) and runs before the relative-URL fast path; backslashes are rejected up front, so the same-domain `ListFirst` no longer needs to treat `\` as a delimiter. Same-origin absolute URLs and genuine relative paths remain allowed (#2898)
- `$getRequestFormat` now rejects non-alphanumeric `url.format` values, preventing path traversal / local file inclusion via the error-template include path in `$runOnError` (#2900)
- `$shouldBlockInProduction()` is now a fail-closed allowlist: the `/wheels/*` dev-UI handlers (routes, env info, CFML REPL, test runner, migrator UI, MCP) 404 unless `application.wheels.environment == "development"` — case-insensitive, matching the gates in `consoleeval.cfm` and `mcp.cfm` — and also when `application.wheels` or its `environment` key is missing. Previously the predicate matched only the literal `"production"`, so any other environment name (`staging`, `qa`, `testing`, `maintenance`, `design`, ...) bypassed the gate when `enablePublicComponent=true` was set, exposing RCE-grade surfaces. `index()` stays ungated per #2233 and `$loadRegistryPackages()` inherits the stricter gate. **Behavioral change:** operators who set `enablePublicComponent=true` in a non-development environment for ad-hoc debugging will now receive 404s on the Tools links — set `environment="development"` (and `enablePublicComponent=true`) to reach them (#2903)
Expand Down
16 changes: 16 additions & 0 deletions vendor/wheels/controller/redirection.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,22 @@ component {
* [category: Miscellaneous Functions]
*/
public boolean function $isSafeRedirectUrl(required string url, required string serverName) {
// Per WHATWG URL parsing: browsers strip embedded ASCII tab/CR/LF and trim leading/trailing
// ASCII whitespace before navigation. Mirror that normalization here so a URL the browser will
// resolve to "//evil.com" cannot pass the same-origin gate via its unstripped form
// (e.g. "<TAB>//evil.com" classifies as a relative path pre-strip but navigates off-domain
// post-strip). Deferred from #2898.
arguments.url = Replace(arguments.url, Chr(9), "", "all");
arguments.url = Replace(arguments.url, Chr(10), "", "all");
arguments.url = Replace(arguments.url, Chr(13), "", "all");
arguments.url = Trim(arguments.url);

// Reject any URL retaining ASCII C0 control characters (NUL through US, or DEL). Browsers
// flag these as validation errors and engine behavior diverges; refuse rather than guess.
if (ReFind("[\x00-\x1F\x7F]", arguments.url)) {
return false;
}

// Reject any URL containing a backslash outright. Browsers normalize backslashes to forward
// slashes ("/\evil.com", "\/evil.com" and "\\evil.com" all navigate to evil.com), so a
// backslash anywhere makes the URL unsafe. Literal backslashes in legitimate URLs should be
Expand Down
65 changes: 65 additions & 0 deletions vendor/wheels/tests/specs/controller/redirectionSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,71 @@ component extends="wheels.WheelsTest" {
application.wheels.allowExternalRedirects = false;
}
})

// Deferred from #2898 — WHATWG URL parsing strips embedded ASCII tab/CR/LF
// and trims leading/trailing whitespace before navigation, so URLs that
// look like same-origin relative paths to the pre-normalization gate
// navigate off-domain once the browser strips. Mirror the strip.

it("throws on redirectTo url with leading tab protocol-relative external domain", () => {
// Browser strips the leading tab and navigates to "//evil.com".
expect(function(){
_controller.redirectTo(url = Chr(9) & "//evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with embedded tab in protocol-relative path", () => {
// Browser strips the embedded tab; result is "//evil.com".
expect(function(){
_controller.redirectTo(url = "/" & Chr(9) & "/evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with leading LF protocol-relative external domain", () => {
expect(function(){
_controller.redirectTo(url = Chr(10) & "//evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with embedded LF in protocol-relative path", () => {
expect(function(){
_controller.redirectTo(url = "/" & Chr(10) & "/evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with leading CR protocol-relative external domain", () => {
expect(function(){
_controller.redirectTo(url = Chr(13) & "//evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with embedded CR in protocol-relative path", () => {
expect(function(){
_controller.redirectTo(url = "/" & Chr(13) & "/evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with leading space protocol-relative external domain", () => {
// Browser strips leading ASCII whitespace per WHATWG.
expect(function(){
_controller.redirectTo(url = " //evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with leading NUL control character", () => {
// NUL (Chr(0)) is a C0 control; engine behavior diverges, so reject outright.
expect(function(){
_controller.redirectTo(url = Chr(0) & "//evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})

it("throws on redirectTo url with embedded vertical-tab in protocol-relative path", () => {
// Vertical tab (Chr(11)) is a C0 control not stripped by browsers but
// can confuse the same-origin classifier. Reject outright.
expect(function(){
_controller.redirectTo(url = "/" & Chr(11) & "/evil.com")
}).toThrow("Wheels.UnsafeRedirect")
})
})
}
}
Loading