BOX-157 #resolve

cbSecurity: validate _securedUrl is for the same domain

Author: lmajano

changelog.md

@@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+
+- **CRITICAL**: Fixed open redirect vulnerability in `_securedURL` handling. The `saveSecuredUrl()` method now validates redirect URLs to ensure they belong to the same host as the current request, preventing attackers from crafting malicious URLs that redirect users to external sites after login. Added `isSafeRedirectUrl()` validation using `java.net.URI` to compare hosts.
+
+### Fixed
+
+-  JWT Handler improperly returns a value causing it to skip ColdBox's RestHandler's response formatting logic.   This results in the entire response object being returned rather than just invoking getDataPacket()
+
 ## [3.5.0] - 2025-10-17
 
 ### Added

interceptors/Security.cfc

@@ -200,11 +200,11 @@ component accessors="true" extends="coldbox.system.Interceptor" {
 	/**
 	 * Listen to module loadings, so we can do module rule registrations
 	 *
-	 * @event        
+	 * @event
 	 * @interceptData
-	 * @rc           
-	 * @prc          
-	 * @buffer       
+	 * @rc
+	 * @prc
+	 * @buffer
 	 */
 	function postModuleLoad( event, interceptData, rc, prc, buffer ){
 		// Is this a cbSecurity Module & not registered
@@ -223,11 +223,11 @@ component accessors="true" extends="coldbox.system.Interceptor" {
 	/**
 	 * Listen to module unloadings, so we can do module rule cleanups
 	 *
-	 * @event        
+	 * @event
 	 * @interceptData
-	 * @rc           
-	 * @prc          
-	 * @buffer       
+	 * @rc
+	 * @prc
+	 * @buffer
 	 */
 	function postModuleUnload( event, interceptData, rc, prc, buffer ){
 		// Is the module registered?
@@ -246,11 +246,11 @@ component accessors="true" extends="coldbox.system.Interceptor" {
 	/**
 	 * Our firewall kicks in at preProcess
 	 *
-	 * @event        
+	 * @event
 	 * @interceptData
-	 * @rc           
-	 * @prc          
-	 * @buffer       
+	 * @rc
+	 * @prc
+	 * @buffer
 	 */
 	function preProcess( event, interceptData, rc, prc, buffer ){
 		// Add SecureView() into the requestcontext
@@ -290,9 +290,9 @@ component accessors="true" extends="coldbox.system.Interceptor" {
 	/**
 	 * Process handler annotation based security rules.
 	 *
-	 * @event        
+	 * @event
 	 * @interceptData
-	 * @currentEvent 
+	 * @currentEvent
 	 */
 	function processAnnotationRules(
 		required event,
@@ -773,6 +773,8 @@ component accessors="true" extends="coldbox.system.Interceptor" {
 
 	/**
 	 * Flash the incoming secured Url so we can redirect to it or use it in the next request.
+	 * This method validates the URL to prevent open redirect vulnerabilities by ensuring
+	 * the URL belongs to the same host as the current request.
 	 *
 	 * @event The event object
 	 */
@@ -784,11 +786,58 @@ component accessors="true" extends="coldbox.system.Interceptor" {
 			translate  : false
 		);
 
+		// Validate the URL to prevent open redirect attacks
+		if ( !isSafeRedirectUrl( securedURL, arguments.event ) ) {
+			// If the URL is not safe, log a warning and use the home page instead
+			if ( log.canWarn() ) {
+				log.warn(
+					"Potential open redirect attempt detected. Invalid secured URL: #securedURL#. Using home page instead.",
+					{ "ip" : variables.cbSecurity.getRealIp(), "url" : securedURL }
+				);
+			}
+			// Use the application's base URL instead
+			securedURL = arguments.event.buildLink( to = "", translate = false );
+		}
+
 		// Flash it and place it in RC as well
 		flash.put( "_securedUrl", securedURL );
 		arguments.event.setValue( "_securedUrl", securedURL );
 	}
 
+	/**
+	 * Validates that a redirect URL is safe by ensuring it belongs to the same host
+	 * as the current request. This prevents open redirect vulnerabilities.
+	 *
+	 * @targetUrl   The URL to validate
+	 * @event The request context
+	 *
+	 * @return True if the URL is safe to redirect to, false otherwise
+	 */
+	private boolean function isSafeRedirectUrl( required string targetUrl, required event ){
+		try {
+			// Parse the URL to validate
+			var urlToValidate = createObject( "java", "java.net.URI" ).init( arguments.targetUrl );
+
+			// If the URL is relative (no host), it's safe
+			if ( isNull( urlToValidate.getHost() ) || !len( urlToValidate.getHost() ) ) {
+				return true;
+			}
+
+			// Get the current request's host for comparison
+			var currentHost = variables.cbSecurity.getRealHost();
+
+			// Compare hosts (case-insensitive)
+			return compareNoCase( urlToValidate.getHost(), currentHost ) == 0;
+		} catch ( any e ) {
+			// If URL parsing fails, consider it unsafe
+            log.warn(
+                "Error parsing URL for redirect validation: #arguments.targetUrl# : #e.message#",
+                e.detail
+            );
+			return false;
+		}
+	}
+
 	/**
 	 * Verifies that the current event is in a given pattern list
 	 *

readme.md

@@ -340,6 +340,8 @@ Once the firewall has the results, and the user is **NOT** allowed access. Then
 - The request will be logged via LogBox
 - If the firewall database logs are enabled, then we will log it in our database logs
 - The current URL will be flashed as `_securedURL` so it can be used in relocations
+  - **Security Note**: The URL is validated to ensure it belongs to the same host as the current request to prevent open redirect attacks
+  - Only same-host URLs or relative URLs are allowed; external URLs will be replaced with the application home page
 - If using a rule, the rule will be stored in `prc` as `cbsecurity_matchedRule`
 - The validator results will be stored in `prc` as `cbsecurity_validatorResults`
 - If the type of invalidation is `authentication` the `cbSecurity_onInvalidAuthentication` interception will be announced

[email protected]

@@ -5,6 +5,7 @@
         "cfengine":"boxlang@1"
     },
     "web":{
+        "host":"0.0.0.0",
         "http":{
             "port":"60299"
         },
@@ -21,7 +22,7 @@
         "javaVersion":"openjdk21_jre",
         "args":"-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=8888"
     },
-    "openBrowser": false,
+    "openBrowser":false,
     "cfconfig":{
         "file":".cfconfig.json"
     },

test-harness/tests/specs/integration/SecuritySpec.cfc

@@ -195,6 +195,7 @@ component extends="coldbox.system.testing.BaseTestCase" appMapping="/root" {
 					} );
 				} );
 			} );
+
 		} );
 	}
 

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

@@ -23,7 +23,7 @@ component extends="coldbox.system.testing.BaseInterceptorTest" interceptor="cbse
 						{ "version" : "6.0.0" },
 						false
 					);
-				mockLogger = createEmptyMock( "coldbox.system.logging.Logger" ).$( "info" );
+				mockLogger = createEmptyMock( "coldbox.system.logging.Logger" ).$( "info" ).$( "warn" )
 				mockController
 					.$( "getSetting" )
 					.$args( "modules" )
@@ -275,6 +275,78 @@ component extends="coldbox.system.testing.BaseInterceptorTest" interceptor="cbse
 					expect( security.getProperty( "firewall" ).rules.inline ).toHaveLength( 1 );
 				} );
 			} );
+
+			describe( "URL validation for open redirect prevention", () => {
+
+				beforeEach( ( currentSpec ) => {
+					mockValidator = mockWireBox.getInstance( settings.firewall.validator );
+					security
+						.$( "getInstance" )
+						.$args( settings.firewall.validator )
+						.$results( mockValidator );
+					security.configure();
+
+					mockSecurityService.$( "getRealHost", "mysite.com" );
+
+					mockEvent = createMock( "coldbox.system.web.context.RequestContext" )
+						.$( "getCurrentRoutedURL", "/account" )
+						.$( "buildLink" )
+						.$args( to = "/account", queryString = "", translate = false )
+						.$results( "/account" )
+						.$( "setValue" );
+
+					mockFlash = createStub().$( "put" );
+					security.$property( "flash", "variables", mockFlash );
+
+                    makePublic( security, "isSafeRedirectUrl" );
+                    makePublic( security, "saveSecuredUrl" );
+				} );
+
+				it( "allows relative URLs without a host", () => {
+                    var result = security.isSafeRedirectUrl( targetUrl = "/account", event = mockEvent );
+					expect( result ).toBeTrue();
+				} );
+
+				it( "allows URLs with the same host", () => {
+					var result = security.isSafeRedirectUrl( targetUrl = "https://mysite.com/account", event = mockEvent );
+					expect( result ).toBeTrue();
+				} );
+
+				it( "blocks URLs with different hosts", () => {
+					var result = security.isSafeRedirectUrl( targetUrl = "https://malicioussite.com/phishing", event = mockEvent );
+					expect( result ).toBeFalse();
+				} );
+
+				it( "blocks URLs with subdomain differences", () => {
+					var result = security.isSafeRedirectUrl( targetUrl = "https://evil.mysite.com/account", event = mockEvent );
+					expect( result ).toBeFalse();
+				} );
+
+				it( "is case-insensitive when comparing hosts", () => {
+					var result = security.isSafeRedirectUrl( targetUrl = "https://MySite.COM/account", event = mockEvent );
+					expect( result ).toBeTrue();
+				} );
+
+				it( "handles invalid URLs gracefully", () => {
+					var result = security.isSafeRedirectUrl( targetUrl = "not a valid url://", event = mockEvent );
+					expect( result ).toBeFalse();
+				} );
+
+				it( "saves secured URL when it is safe", () => {
+                    mockEvent.$( "getCurrentRoutedURL", "/account" );
+					mockEvent
+                        .$( "buildLink" )
+						.$args( to = "/account", queryString = cgi.QUERY_STRING, translate = false )
+						.$results( "/account" );
+
+					security.saveSecuredUrl( mockEvent );
+
+					// Verify flash was called with the safe URL
+					expect( mockFlash.$callLog().put ).toHaveLength( 1 );
+					expect( mockFlash.$callLog().put[ 1 ][ 1 ] ).toBe( "_securedUrl" );
+					expect( mockFlash.$callLog().put[ 1 ][ 2 ] ).toBe( "/account" );
+				} );
+			} );
 		} );
 	}