8000 Redirect URLs not starting with '/' get dropped · Issue #46533 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

Redirect URLs not starting with '/' get dropped #46533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
martinmaehlmannchefkoch opened this issue May 31, 2022 · 2 comments
Closed

Redirect URLs not starting with '/' get dropped #46533

martinmaehlmannchefkoch opened this issue May 31, 2022 · 2 comments
Labels
Bug Security Status: Needs Review

Comments

@martinmaehlmannchefkoch
Copy link

Symfony version(s) affected

4.4.42

Description

By fixing bug #46317 a new bug was introduced. If a _target_path starts with 'http' or 'https' it gets dropped and the default location is returned. Using the Referer should still work.

How to reproduce

In a login form allowing _target_path, modify the value to send a value which starts with http or https.

Possible Solution

Simply modify the check to see, if it is a valid URL. Something like this could work:

if (\is_string($targetUrl) && (str_starts_with($targetUrl, '/') || filter_var($url, FILTER_VALIDATE_URL) )) {
    return $targetUrl;
}

Additional Context

No response
@xabbuh xabbuh added the Security label Jun 1, 2022
@xabbuh
Copy link
Member
xabbuh commented Jun 1, 2022

Would you like to give this a try and send a PR fixing this?

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 1, 2022 •
edited
Loading

Given the check in HttpUtils::generateUri(), I would go with a simple str_starts_with($targetUrl, '/') || str_starts_with($targetUrl, 'http'), in both DefaultAuthenticationFailureHandler and DefaultAuthenticationSuccessHandler.

PR welcome indeed.

UFTimmy pushed a commit to UFTimmy/symfony that referenced this issue Jul 26, 2022
[Security] Allow redirect after login to absolute URLs
b9901aa 
Fixes symfony#46533
@UFTimmy UFTimmy mentioned this issue Jul 26, 2022
Merged
@fabpot fabpot closed this as completed Jul 29, 2022
fabpot added a commit that referenced this issue Jul 29, 2022
@fabpot
bug #47069 [Security] Allow redirect after login to absolute URLs (Ti…
dfa765d 
…m Ward)

This PR was merged into the 4.4 branch.

Discussion
----------

[Security] Allow redirect after login to absolute URLs

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #46533
| License       | MIT
| Doc PR        |

Fixes the regression introduced by #46317, and once again allows absolute URLs to be the target of authentication redirection, as specified in the documentation (https://symfony.com/doc/current/security/form_login.html#changing-the-default-page)

Commits
-------

b9901aa [Security] Allow redirect after login to absolute URLs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Security Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @nicolas-grekas @xabbuh @carsonbot @martinmaehlmannchefkoch
0