5

I am developing a PHP web app and I'm in the process of implementing a redirection functionality.

I know that redirection can be dangerous when it can be set on the client side and I use ../../ as a prefix for redirection.

Is this enough to make sure one can only redirect to internal pages? I tried it with $_POST['redirurl'] containing stuff like http://www.evil.com and ; http://www.evil.com but non worked.

Is this safe?

if(isset($_POST['redirurl'])){
    $redir='Location: ../../'.$_POST['redirurl'];
    header($redir);
}
Purefan
  • 3,560
  • 19
  • 26
ozzi-
  • 53
  • 3

2 Answers2

2

You should never allow redirection to a relative path that can be manipulated on the client side. As you say, it's dangerous as it potentially allows access to files outside of your webroot. A better approach is to "tag" or name the target pages and redirect to them inside your code.

For example, when a user triggers a form, let it post redirpage with a certain value, like dashboard to redirect a user to a dashboard page. Then in your code, just check for a supported page name:

switch ($_POST['redirpage']) {
    case 'dashboard':
       header('Location: http://www.example.com/dashboard/');
       break;
    case 'account':
       header('Location: http://www.example.com/account/');
       break;
    default:
        // This fallback will be triggered when an "unknown" target page is set
        header('Location: http://www.example.com/');
}

That way, you control what redirects can be done and where they end up.

Oldskool
  • 400
  • 1
  • 10
  • Thank you very much for your explanation! I will most likely use a solution like yours or set the variable server sided into the php session, thus not allowing the client to interfere. – ozzi- Feb 02 '16 at 09:15
  • 4
    Setting a Location header does not allow access to files outside of the web root. In fact, it doesn't grant access to any files that a malicious user couldn't already access by changing the address in his browser's address bar. What you're thinking of is passing query strings to functions such as `file_get_contents()` or `include`, which is indeed incredibly unsafe. – Duroth Feb 02 '16 at 12:49
  • 1
    @Duroth Hmm, good point. Although it can become problematic when working on subdomains, doing a Location header starting with `../` on a subdomain will cause you to be redirected to the primary domain instead. That might also be undesirable. In any case, when working with locations, whether URI's or local file paths, it's never a good idea to rely on client-side input. – Oldskool Feb 03 '16 at 09:02
1

As Oldskool points out, there are other problems with this strategy. But you might also be vulnerable to header injection.

From the changelog of header() PHP manual:

5.1.2: This function now prevents more than one header to be sent at once as a protection against header injection attacks.

So on PHP versions before 5.1.2 you could send something containing a newline to set other headers (or set the location header again). That is a security risk. And even if you are on a newer PHP version, what if somebody uses your code on an old server for some reason you can not foresee? Mitigate this problem by calling rawurlencode() on the parameter.

Anders
  • 64,406
  • 24
  • 178
  • 215