fix: use resolveSafeRedirect() in Login POST handler to prevent Open Redirect (CWE-601) by javokhir-sec · Pull Request #3658 · Leantime/leantime

javokhir-sec · 2026-07-17T12:54:14Z

Description

Fixes CWE-601 (URL Redirection to Untrusted Site) in the Login controller.

Vulnerability

The Login::post() method redirects users to a user-controlled redirect parameter after login without validation. An attacker can craft a link that appears to go to the legitimate Leantime instance but redirects the victim to a phishing site after login.

Impact (CVSS 6.1)

Phishing attacks — attacker creates convincing links pointing to real Leantime
Credential harvesting via fake login pages
Session token theft via redirect to attacker-controlled domain

Fix

Used the existing resolveSafeRedirect() helper (already used elsewhere in the codebase) to validate the redirect URL. Falls back to dashboard if the redirect parameter is invalid.

PoC

https://TARGET/login?redirect=https://attacker.com/phishing

Researcher: Javokhir Tursunboyev (@javokhir-sec)
Disclosure: Responsible, with fix PR

CLAassistant · 2026-07-17T12:54:33Z

All committers have signed the CLA.

…redirect The Login POST handler read redirectUrl directly with only FILTER_SANITIZE_URL and urldecode(), neither of which validates the redirect hostname. An attacker could craft a login form with a redirectUrl pointing to a phishing site. The GET handler already used resolveSafeRedirect() which validates that the redirect target is an internal URL. This fix aligns the POST handler with the same safe pattern. CWE-601 (URL Redirection to Untrusted Site) CVSS:3.1 AV:N/AC:L/PR:N/UI:R/S:C/C:L/I:L/A:N (6.1) Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

Pull request overview

This PR addresses an Open Redirect (CWE-601) risk in the Auth login flow by routing the post-login redirect through the existing AuthService::resolveSafeRedirect() helper, defaulting to the dashboard when the target is invalid.

Changes:

Updated the Login POST handler to validate the user-supplied redirect target via resolveSafeRedirect().
Removed inline sanitization / fallback logic in Login::post() in favor of the shared helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

-                $redirectUrl = BASE_URL.'/dashboard/home';
-            }
+
+            $redirectUrl = $this->authService->resolveSafeRedirect($_POST['redirectUrl'] ?? null);


marcelfolaron · 2026-07-17T14:11:38Z

Status: ready-pending-CI · Priority: P2 (open redirect / phishing) · Next action: confirm resolveSafeRedirect handles null + add a test · Owner: @javokhir-sec (fix), review gate @marcelfolaron / @broskees

Automated maintainer review (advisory only — not an approval; merge authority stays with @marcelfolaron / @broskees). Reviewed at head 5d0cafcd.

Intent: Closes CWE-601 in Login::post() by routing the post-login redirect through the existing Auth::resolveSafeRedirect() helper instead of trusting $_POST['redirectUrl'] verbatim.

This is the cleanest of the batch — it reuses an existing, presumably-tested helper rather than hand-rolling validation, and it also removes the duplicated empty-URL default. Two things to verify:

resolveSafeRedirect(null) must be safe. Login.php L67: $this->authService->resolveSafeRedirect($_POST['redirectUrl'] ?? null). The old code special-cased the missing-field branch to BASE_URL.'/dashboard/home' (the comment noted Frontcontroller::redirect('') throws "Cannot redirect to an empty URL"). Confirm resolveSafeRedirect() returns the dashboard default (not '' / not null) when passed null — otherwise non-browser callers that omit the field regress to a 500.
Same helper on the GET path? The GET handler already used resolveSafeRedirect per that comment; just confirm both entry points now share it so there's no second unguarded redirect sink.
Verify the helper actually blocks off-host absolute URLs (https://attacker.com, protocol-relative //attacker.com, and \/\//backslash tricks). The fix is only as strong as resolveSafeRedirect — worth a one-line confirmation of what it allows.

Acceptance checklist (must pass before merge):

CI green: Pint + PHPStan level 5 + full unit suite.
A test: redirect=https://attacker.com (and //attacker.com) falls back to the dashboard; a same-host relative path is preserved; null/missing field yields the dashboard default with no exception.
Backward-compat confirmed for the no-redirectUrl (API/curl) caller — no "empty URL" 500.

CI: Can't read checks from here — confirm in the UI. If resolveSafeRedirect already carries its own coverage, cite it and the redirect-specific test gap is satisfied.

marcelfolaron · 2026-07-19T13:49:54Z

Copilots comment is valid. Oftentimes we will have the full absolute URL of the leantime instance in the var, so this would prevent the redirect. It needs to strip the base url first (or check and allow)

Address maintainer review feedback from PR Leantime#3658: The login form often submits a full absolute URL (e.g. https://my-leantime.com/dashboard/home) in the redirectUrl hidden field. Previously resolveSafeRedirect() treated any valid URL (including same-origin absolute URLs) as external and rejected it, falling back to the default dashboard redirect. Fix: strip the BASE_URL prefix before the open-redirect guard so same-origin absolute URLs are treated identically to their relative counterparts. External URLs are still blocked. New tests: - Same-origin absolute URL → allowed (strips BASE_URL) - Same-origin absolute URL with deep path → allowed - URL-encoded same-origin absolute URL → allowed (urldecode first) - External URL with BASE_URL-like path prefix → rejected Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

@@ -488,12 +488,20 @@
        if ($redirect !== null && trim($redirect) !== '' && trim($redirect) !== '/') {
            $url = urldecode($redirect);


+            if (filter_var($url, FILTER_VALIDATE_URL) === false) {
+                $redirectUrl = BASE_URL.'/'.ltrim($url, '/');
+            } elseif (in_array($url, ['/auth/logout'], true)) {
+                $redirectUrl = BASE_URL.$url;
            }


- Replace urldecode() with rawurldecode() to prevent + sign corruption in redirect URL query strings - Remove dead elseif branch for /auth/logout: the path is a relative URL (not a valid absolute URL), so it always hits the FILTER_VALIDATE_URL === false branch first, producing the same BASE_URL result Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

+        // URL-encoded same-origin absolute URL — urldecode is called first,
+        // then BASE_URL is stripped.
+        $this->assertSame(


+            // Strip the application base URL when present so that same-origin
+            // absolute URLs (e.g. https://my-leantime.com/dashboard/home) are
+            // treated the same as their relative counterparts. The login form
+            // may submit a full absolute URL in the redirectUrl field.
+            if (str_starts_with($url, BASE_URL)) {


javokhir-sec requested a review from a team as a code owner July 17, 2026 12:54

javokhir-sec requested review from broskees and Copilot and removed request for a team and Copilot July 17, 2026 12:54

Copilot AI review requested due to automatic review settings July 17, 2026 13:28

javokhir-sec force-pushed the fix/open-redirect-login branch from f33ad6b to d9b3737 Compare July 17, 2026 13:28

javokhir-sec force-pushed the fix/open-redirect-login branch from d9b3737 to 5d0cafc Compare July 17, 2026 13:28

Copilot started reviewing on behalf of javokhir-sec July 17, 2026 13:28 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread app/Domain/Auth/Controllers/Login.php

$redirectUrl = BASE_URL.'/dashboard/home';

}

$redirectUrl = $this->authService->resolveSafeRedirect($_POST['redirectUrl'] ?? null);

Copilot AI review requested due to automatic review settings July 20, 2026 07:22

Copilot started reviewing on behalf of javokhir-sec July 20, 2026 07:23 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 20, 2026 09:13

Copilot started reviewing on behalf of javokhir-sec July 20, 2026 09:13 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: use resolveSafeRedirect() in Login POST handler to prevent Open Redirect (CWE-601)#3658

fix: use resolveSafeRedirect() in Login POST handler to prevent Open Redirect (CWE-601)#3658
javokhir-sec wants to merge 3 commits into
Leantime:masterfrom
javokhir-sec:fix/open-redirect-login

javokhir-sec commented Jul 17, 2026

Uh oh!

CLAassistant commented Jul 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

marcelfolaron commented Jul 17, 2026

Uh oh!

marcelfolaron commented Jul 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -488,12 +488,20 @@
		if ($redirect !== null && trim($redirect) !== '' && trim($redirect) !== '/') {
		$url = urldecode($redirect);

Uh oh!

Uh oh!

Conversation

javokhir-sec commented Jul 17, 2026

Description

Vulnerability

Impact (CVSS 6.1)

Fix

PoC

Uh oh!

CLAassistant commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

marcelfolaron commented Jul 17, 2026

Uh oh!

marcelfolaron commented Jul 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Jul 17, 2026 •

edited

Loading