From 8b872363af6124308df2ac0efeff80ebe8338b43 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 21:36:57 +0000 Subject: [PATCH 1/3] Initial plan From 78ac5cb062c6e84ad8a79a70b201797b5998972e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 21:41:52 +0000 Subject: [PATCH 2/3] Fix open redirect vulnerability in LoginController Co-authored-by: ljonesfl <1099983+ljonesfl@users.noreply.github.com> --- src/Cms/Controllers/Auth/LoginController.php | 44 ++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/Cms/Controllers/Auth/LoginController.php b/src/Cms/Controllers/Auth/LoginController.php index e755a10..5fd686e 100644 --- a/src/Cms/Controllers/Auth/LoginController.php +++ b/src/Cms/Controllers/Auth/LoginController.php @@ -44,6 +44,36 @@ public function __construct( ?Application $app = null ) $this->_CsrfManager = new CsrfTokenManager( $this->_SessionManager ); } + /** + * Validate redirect URL to prevent open redirect vulnerabilities + * + * @param string $url The URL to validate + * @return bool True if the URL is valid, false otherwise + */ + private function isValidRedirectUrl( string $url ): bool + { + // Only allow relative URLs or same-origin absolute URLs + if( empty( $url ) ) + { + return false; + } + + // Reject URLs with schemes (http://, https://, javascript:, etc.) + if( preg_match( '#^[a-z][a-z0-9+.-]*:#i', $url ) ) + { + return false; + } + + // Reject protocol-relative URLs (//example.com) + if( str_starts_with( $url, '//' ) ) + { + return false; + } + + // Must start with / for internal path + return str_starts_with( $url, '/' ); + } + /** * Show login form */ @@ -56,13 +86,18 @@ public function showLoginForm( array $Parameters ): string exit; } + $requestedRedirect = $_GET['redirect'] ?? '/admin/dashboard'; + $redirectUrl = $this->isValidRedirectUrl( $requestedRedirect ) + ? $requestedRedirect + : '/admin/dashboard'; + $ViewData = [ 'Title' => 'Login | ' . $this->getName(), 'Description' => 'Login to ' . $this->getName(), 'CsrfToken' => $this->_CsrfManager->getToken(), 'Error' => $this->_SessionManager->getFlash( 'error' ), 'Success' => $this->_SessionManager->getFlash( 'success' ), - 'RedirectUrl' => $_GET['redirect'] ?? '/admin/dashboard' + 'RedirectUrl' => $redirectUrl ]; return $this->renderHtml( @@ -112,8 +147,11 @@ public function login( array $Parameters ): string $this->_SessionManager->flash( 'success', 'Welcome back!' ); // Redirect to intended URL or dashboard - $RedirectUrl = $_POST['redirect_url'] ?? '/admin/dashboard'; - header( 'Location: ' . $RedirectUrl ); + $requestedRedirect = $_POST['redirect_url'] ?? '/admin/dashboard'; + $redirectUrl = $this->isValidRedirectUrl( $requestedRedirect ) + ? $requestedRedirect + : '/admin/dashboard'; + header( 'Location: ' . $redirectUrl ); exit; } From 806b4a4b84f894f88323cdc5e21a36c2b38e2d5b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 21:44:01 +0000 Subject: [PATCH 3/3] Address code review feedback: improve redirect validation Co-authored-by: ljonesfl <1099983+ljonesfl@users.noreply.github.com> --- src/Cms/Controllers/Auth/LoginController.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Cms/Controllers/Auth/LoginController.php b/src/Cms/Controllers/Auth/LoginController.php index 5fd686e..c782ac6 100644 --- a/src/Cms/Controllers/Auth/LoginController.php +++ b/src/Cms/Controllers/Auth/LoginController.php @@ -22,6 +22,11 @@ class LoginController extends Content private SessionManager $_SessionManager; private CsrfTokenManager $_CsrfManager; + /** + * Default redirect URL after login + */ + private const DEFAULT_REDIRECT_URL = '/admin/dashboard'; + public function __construct( ?Application $app = null ) { parent::__construct( $app ); @@ -52,14 +57,14 @@ public function __construct( ?Application $app = null ) */ private function isValidRedirectUrl( string $url ): bool { - // Only allow relative URLs or same-origin absolute URLs + // Only allow internal relative paths if( empty( $url ) ) { return false; } // Reject URLs with schemes (http://, https://, javascript:, etc.) - if( preg_match( '#^[a-z][a-z0-9+.-]*:#i', $url ) ) + if( preg_match( '/^[a-zA-Z][a-zA-Z0-9+.-]*:/', $url ) ) { return false; } @@ -82,14 +87,14 @@ public function showLoginForm( array $Parameters ): string // If already logged in, redirect to dashboard if( $this->_AuthManager->check() ) { - header( 'Location: /admin/dashboard' ); + header( 'Location: ' . self::DEFAULT_REDIRECT_URL ); exit; } - $requestedRedirect = $_GET['redirect'] ?? '/admin/dashboard'; + $requestedRedirect = $_GET['redirect'] ?? self::DEFAULT_REDIRECT_URL; $redirectUrl = $this->isValidRedirectUrl( $requestedRedirect ) ? $requestedRedirect - : '/admin/dashboard'; + : self::DEFAULT_REDIRECT_URL; $ViewData = [ 'Title' => 'Login | ' . $this->getName(), @@ -147,10 +152,10 @@ public function login( array $Parameters ): string $this->_SessionManager->flash( 'success', 'Welcome back!' ); // Redirect to intended URL or dashboard - $requestedRedirect = $_POST['redirect_url'] ?? '/admin/dashboard'; + $requestedRedirect = $_POST['redirect_url'] ?? self::DEFAULT_REDIRECT_URL; $redirectUrl = $this->isValidRedirectUrl( $requestedRedirect ) ? $requestedRedirect - : '/admin/dashboard'; + : self::DEFAULT_REDIRECT_URL; header( 'Location: ' . $redirectUrl ); exit; }