Skip to content

Commit 009d388

Browse files
Copilotlmajano
andcommitted
Fix isSafeRedirectUrl host comparison for non-default ports
- Strip port from getRealHost() before comparing with URI.getHost() - Add test cases for non-default port scenarios - Fixes issue where valid same-host redirects were rejected during dev Co-authored-by: lmajano <137111+lmajano@users.noreply.github.com>
1 parent 715035e commit 009d388

File tree

2 files changed

+36
-1
lines changed

2 files changed

+36
-1
lines changed

interceptors/Security.cfc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,9 @@ component accessors="true" extends="coldbox.system.Interceptor" {
827827
}
828828

829829
// Get the current request's host for comparison
830-
var currentHost = variables.cbSecurity.getRealHost();
830+
// Normalize host: urlToValidate.getHost() does not include port
831+
// Strip port from .getRealHost() for compare
832+
var currentHost = listFirst( variables.cbSecurity.getRealHost(), ":" );
831833

832834
// Compare hosts (case-insensitive)
833835
return compareNoCase( urlToValidate.getHost(), currentHost ) == 0;

test-harness/tests/specs/unit/SecurityTest.cfc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,39 @@ component extends="coldbox.system.testing.BaseInterceptorTest" interceptor="cbse
342342
expect( result ).toBeTrue();
343343
} );
344344

345+
it( "allows URLs with non-default ports when host matches", () => {
346+
// Simulate getRealHost returning host with port (e.g., during dev)
347+
mockSecurityService.$( "getRealHost", "127.0.0.1:61910" );
348+
349+
var result = security.isSafeRedirectUrl(
350+
targetUrl = "http://127.0.0.1:61910/account",
351+
event = mockEvent
352+
);
353+
expect( result ).toBeTrue();
354+
} );
355+
356+
it( "allows URLs with different ports when host matches", () => {
357+
// getRealHost returns host with port
358+
mockSecurityService.$( "getRealHost", "mysite.com:8080" );
359+
360+
// URL has a different port, but same host
361+
var result = security.isSafeRedirectUrl(
362+
targetUrl = "https://mysite.com:9000/account",
363+
event = mockEvent
364+
);
365+
expect( result ).toBeTrue();
366+
} );
367+
368+
it( "blocks URLs with different hosts even with same port", () => {
369+
mockSecurityService.$( "getRealHost", "mysite.com:8080" );
370+
371+
var result = security.isSafeRedirectUrl(
372+
targetUrl = "https://malicious.com:8080/phishing",
373+
event = mockEvent
374+
);
375+
expect( result ).toBeFalse();
376+
} );
377+
345378
it( "handles invalid URLs gracefully", () => {
346379
var result = security.isSafeRedirectUrl( targetUrl = "not a valid url://", event = mockEvent );
347380
expect( result ).toBeFalse();

0 commit comments

Comments
 (0)