3

If somebody were accessing a protected page while logged out, they would be redirected to a login page:

/login?next=/something/123/manage?sort=desc

If a successful login, they should be redirected to the page they have been intending to view:

if verify_creds(user, password):
    login(user)
    return redirect(request.args['next'])

How to verify that an attacker cannot use the next parameter to visit a phishing site? Is it sufficient to check if next starts with /, ie a relative link or should I try to check if its a valid path in my application?

Jesvin Jose
  • 519
  • 1
  • 5
  • 10

2 Answers2

3

Is it sufficient to check if next starts with /, ie a relative link

No, it's not sufficient. Take this script as an example:

<?php header('Location: ' . $_GET['next']); ?>

An attacker could trigger an open redirect via redirect.php?next=//example.com/. You browser interprets this as an absolute URL using the same protocol.

should I try to check if its a valid path in my application?

That would be the most secure solution. If checking valid paths is not an option, you should instead make sure the URL starts with /[a-zA-Z0-9] (that is one slash and one alphanumeric character).

Checking that it starts with / and not with //, as suggested by @BenoitEsnard, might be secure in some contexts but it's not always sufficient. Take this example (and ignore the obvious XSS problem for argument's sake):

<meta http-equiv="refresh" content="0; url=<?php echo $_GET['next']; ?>">

Here, an attacker could achieve a redirect via redirect.php?next=/%26sol;example.com/ which HTML-encodes the second / and wouldn't be picked up by a filter that checks for a // beginning.

The OWASP cheat sheet on unvalidated redirects has some additional advice.

Arminius
  • 44,770
  • 14
  • 145
  • 139
2

No, it's not enough to check that next starts with /.

Protocol-relative URLs, like //example.org could be used to bypass this check, and redirect the user to an external domain.

Checking that the URL starts with / and doesn't start with // should fully mitigate this issue.

Benoit Esnard
  • 14,694
  • 7
  • 69
  • 69