Skip to content

$isSafeRedirectUrl: leading-whitespace / embedded control-char bypass (deferred from #2898) #2972

Closed

Description

@bpamiri

Problem

PR #2898 hardened $isSafeRedirectUrl() against backslash and schemeless-authority bypasses and explicitly deferred one vector "for a separate finding" — which was never filed (caught by the 2026-06-10 coverage audit; the only security-shaped untracked item).

vendor/wheels/controller/redirection.cfc ~:177-199 performs no trim or control-character normalization before classification. Per the WHATWG URL spec, browsers strip leading/trailing C0 controls and whitespace and remove embedded tab/CR/LF before parsing — so a value like \t//evil.com or /\n/evil.com is classified by the gate as a safe local path but navigates to //evil.com after browser normalization.

Impact

Open-redirect residual on redirectTo(url=params.userInput) with default allowExternalRedirects=false — phishing / token-leak vector, same class as #2898.

Suggested approach

Normalize before classification: reject (or strip-then-reclassify) any candidate containing C0 controls; trim per the WHATWG algorithm. Mirror #2898's test-vector style with Chr(9)/Chr(10)/Chr(13) cases.

Acceptance criteria

  • All control-char/whitespace vectors classified unsafe (or normalized identically to browser behavior before the existing checks).
  • Specs for the listed vectors red against pre-fix code.

Source

Deferred-scope note in PR #2898; internal framework review 2026-06-09; coverage audit 2026-06-10.

Activity

  1. wheels-bot commented on Jun 10, 2026

    @wheels-bot
    Contributor

    Wheels Bot — Triage

    Classified as bug.

    Suspected layer

    controller — see .ai/wheels/controllers/ for canonical patterns. The affected function is $isSafeRedirectUrl() in vendor/wheels/controller/redirection.cfc:176-217, a same-file sibling of redirectTo().

    Fix sketch

    Normalize the URL input before the existing classification branches in $isSafeRedirectUrl(). Per the issue: trim leading/trailing C0 control characters and whitespace per the WHATWG URL algorithm, then either reject any candidate still containing embedded tab/CR/LF (Chr(9)/Chr(10)/Chr(13)) or strip-and-reclassify so post-normalization input matches what the browser will navigate to. Add specs alongside vendor/wheels/tests/specs/controller/ (or the security spec dir if #2898's vectors live there) mirroring the Chr(9)/Chr(10)/Chr(13) and \t//evil.com / /\n/evil.com cases listed in the report. Single-file fix in vendor/wheels/controller/redirection.cfc plus a spec file.

    Confidence: medium

    Report is exceptionally specific (function, line range, vectors, suggested approach, prior-art PR #2898), but $isSafeRedirectUrl() is the security boundary for open-redirect prevention — downgrading from high per the security/auth-flow rule so propose-fix's safety net and the reviewer panel re-check the normalization choice (reject vs. strip-then-reclassify) and confirm the spec vectors are comprehensive.

    <!-- wheels-bot:triage:2972 -->
    <!-- wheels-bot:triage-class:bug -->
    <!-- wheels-bot:triage-confidence:medium -->

  2. wheels-bot commented on Jun 10, 2026

    @wheels-bot
    Contributor

    Wheels Bot — Fix proposed

    Draft PR: #2979

    Spec: vendor/wheels/tests/specs/controller/redirectionSpec.cfc — 9 new specs covering Chr(9)/Chr(10)/Chr(13) in leading and embedded positions, leading ASCII space, leading Chr(0) (NUL), and embedded Chr(11) (vertical tab); failing → passing on Lucee 7 + SQLite.
    Implementation: vendor/wheels/controller/redirection.cfc — WHATWG-style normalization (strip embedded tab/CR/LF, trim leading/trailing whitespace, reject any remaining C0 control char) inserted at the top of $isSafeRedirectUrl(), ahead of the existing backslash check from #2898.

    A human review is required before merge. Reviewer A and Reviewer B will weigh in shortly. Doc updates (MDX guides, .ai/wheels/, CLAUDE.md) are handled separately by bot-update-docs.yml.

  3. added a commit that references this issue on Jun 10, 2026
    59076c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions