Is it safe to redirect to URL parameter without filtering?
The short answer
No. This is not safe, and should not be done. In fact, this is the last one of OWASP Top 10:
A10. Unvalidated Redirects and Forwards
Web applications frequently redirect and forward users to other pages and websites, and use untrusted data to determine the destination pages. Without proper validation, attackers can redirect victims to phishing or malware sites, or use forwards to access unauthorized pages.
Phishing and spreading malware
An attacker could craft an URL to redirect to any page they want, and then spread it everywhere:
http://trusted.com/index.php?redirect=http%3A%2F%2Fmalicious.com
The malicious URL could be obfuscated to avoid detection by the user by URL encoding all character so you get a string like %68%74%74%70%3A%2F%2F%6D%61%6C%69%63%69%6F%75%73%2E%63%6F%6D
.
Now a user clicking what looks like an ordinary safe link to your site might end up anywhere:
- On a site looking exactly like yours, asking for username and password. Boom, accounts stolen.
- A site spreading malware by drive by downloads. You could try to claim this is not your problem, since it is technically not your site spreading it. I am not sure the victim would agree.
- Just anything. Do you want users clicking links to your site and ending up on illegal or NSFW or NSFL content?
If this is done after the user logins, as you hint at in the question, it is even more dangerous since actually being on your page entering the credentials strengthens the users belief that he is actually on your site. It would be so easy to serve an "incorrect password, try again" type of page after the login.
Header injection
As already stated by Steffen Ullrich in his answer to this question and me in this answer to another question, on PHP versions older than 5.1.2 this could be used for header injection.
What to do instead
Whitelisting or validation
As so often, the solution is to whitelist. Here is an PHP example:
$param = $_GET["redirect"];
$allowed = array(
"index" => "index.php",
"blog" => "blog/index.php",
// ...and so on...
);
$redirect = isset($allowed[$param]) ? $allowed[$param] : "index.php";
header('Location: '.$redirect);
This will be tedious if there are many pages you want to be able to redirect to. Depending on what your URLs look like, you could validate URLs for your own site. For instance if they are on the form http://trutsted.com/?pageid=1234
you could accept only the pagied to redirect to and verify that it is in fact numeric.
Referer check
If you only want the redirect links to work from within your own site, you could check the HTTP referer heading of the request. This might run into some issues if you serve your site over both HTTP and HTTPS, since if you link from a HTTP page to an HTTPS page the referer header will not be sent.
Here is some example code:
$headers = apache_request_headers();
$referer = isset($headers['referer']) ? parse_url($headers['referer'], PHP_HOST) : "";
if($referer == "trusted.com") {
//Do the redirect. If you want your code to be safe on old PHP versions,
//you will need to filter out newlines or just URL encode the URL.
}
else {
//Inform the user of the problem or just redirect to your index page.
}
More reading
- OWASP cheat sheet on unvalidated redirects. (One of the examples on how not to do it here is alwmost identical to how you did it in your question.)
- Troy Hunt has a great piece on this topic.
Current versions of PHP detect and prevent newline injections in the header function, see How to avoid HTTP Header Injection (new lines characters). In older versions pf PHP you could probably do something like
login.php?redirect=%0D%0A%0D%0A<script>...
Which would break out of the header and result in
Location:
<script>...
And your script then could do whatever it wants, including accessing and forwarding session cookies for session hijacking.
That does not mean that your script is safe with current PHP. Redirecting to arbitrary URL's is never a good idea so you should restrict newURL to the URL's you expect for redirection.
To add to what others have said :
If you have a set of known URL's to redirect to (that you could map to an identifier), it would be much better to allow only known identifiers in the "redirect" parameter value. Then you can map the identifier to your safe, known, URL.
Thanks to such a technique "all your troubles" go away.
Of course if the value of the URL is 100% arbitrary for your users, or if you don't have the set of all possible URL's, then this cannot apply. But if your URL's correspond to known pages that you have control over, go for it !