6

Can I use a posted value in a PHP redirect header safetly without checking it:

header( "Location: $base".$_POST['return'] ); // $base is set to the base of the site

(If the user would somehow manipulate return to a file that doesn't exist it would simply return a nice 404 message)

Is there any danger in doing this? Is there anything the user can set it to that can compromise the system or in any way do damage?

Chaim
  • 2,109
  • 4
  • 27
  • 48

3 Answers3

3

The header() function is no longer vulnerable to HTTP Response Splitting. The only vulnerability you have to worry about is OWASP a10 - unvalidated redirects and forwards.

Providing a $base of anything other than the null string will prevent an attacker from forwarding a user to a remote domain, which could be useful for Phishing. Redirecting to the same domain could be useful to the attacker if are checking the referer as a form of CSRF prevention, but that is a weak form of protection that you really shouldn't be using anyway. Even with a base, the attacker can change the path by supplied a value like: /../../../admin.php, but this is still relative to the originating domain which in most cases is safe.

One great way to deal with unvalidated redirects is to avoid the problem entirely by not using a REQUEST variable. Instead store it in a $_SESSION['redirect'], and use that for the next request. To be a bit more robust, you could say $_SESSION['redirect_checkout'], or something page specific.

Another option is to use a white list, create a list of all values you would like to accept, and make sure the user supplied value is in your list.

sanmai
  • 29,083
  • 12
  • 64
  • 76
rook
  • 66,304
  • 38
  • 162
  • 239
  • Great answer, thanks. Do you think it warrants checking that the `return` value is actually a file that exists on my server? (Also remember that wherever the unvalidated redirect goes, it will go someplace on my server because of the `$base`.) If it's relevant, the context is a login page that redirects back to the page the user was on before they were taken to login. – Chaim Nov 27 '11 at 17:09
  • No, definitely not! PHP doesn't check of a script file exists. In fact, a script need not even exist and user may still get a result. @Rook explained, that header() does take into account, that ill-formatted parameters get passed in - in case your PHP is younger than 5.1.2 – SteAp Nov 27 '11 at 18:41
  • @StefanPantke Ok, I understand, my comment was in regard to @Rook's second point about unvalidated redirects. What I meant was should my script check that the value of `return` is legitimate in terms of the architecture of my website. – Chaim Nov 27 '11 at 18:48
  • Ah, OK. Since you want to present only well-defined page, I propose to check the value. Not due to security reasons. But due to user-experience reasons. – SteAp Nov 27 '11 at 18:50
  • @StefanPantke If the user modifies the value of return than it's his fault that he get's a 404 error. If there's no security risk involved than there's no need to check anything. – Chaim Nov 27 '11 at 19:00
  • If an application is bullet-proof, no problem may happen. Granted! I propose a defensive overall approach: Don't assume, that no problem may ever happen. You might argue, that my approach is over-defensive. I argue, that a good developer can't be defensive enough. – SteAp Nov 27 '11 at 19:03
  • Other example of defensive code: E.g. I always check pre- and post-conditions of method calls using assertions. 20 years ago, others argued against this. 'Hm, extra lines of code...'. Today, it's quite common. – SteAp Nov 27 '11 at 19:07
  • @Rook great, thanks for the session variable idea. I think that's the approach I'm going to go with. – Chaim Nov 27 '11 at 19:22
2

Yes, absolutely! Don't trust any $_GET or $_POST values anytime!

Suppose a third party site posts the form. It may post whatever address.

A simple solution would be not to include the address, but a md5() hash of the address into the form. Once the form gets posted, it's the task of your script to map the hash to an actual address and then emit the Location header.

My other post might be of interest.

You might argue, that your app is bullet-proof. Why shouldn't I pass an URL directly?

In fact, even well-designed applications aren't that bullet-proof. Step back and try to remember your last 'Ah, I forgot something. Let's fix it' event.

Did you check at each point control each and any condition?

  • User clicks on a web-form submit-button twice. Thus controller runs twice.
  • User presses F5 an resubmits the last updating controller twice.
  • User somehow manipulated parameters and a controller gets called with off values passed in.

Therefore, I propose to not pass links or other parameters directly or unprotected / unvalidated.

@Col. Shrapnel: I'm fully aware, that any URL at any point could be submitted to a web-app. That's trivial.

Nevertheless, at a given point of control flow, there are certain acceptable next states of control flow.

To ensure, that only those next control-flow states get reached, I propose to validate.

A more general approach

In fact, my recently updated internal framework never passes any parameters as GET or POST parameters from request to request. All parameters are saved and retrieved from a user session [inside a so called Flow, a part of a bigger control flow].

Using the framework, only one single parameter - the FlowID - gets passed around. If the framework doesn't find the FlowID in the session's flow-store, the framework throws an exception and the dispatcher emits an error message.

Community
  • 1
  • 1
SteAp
  • 11,853
  • 10
  • 53
  • 88
  • 3
    Why? Can you give me an example of something someone can do to cause any damage? (I'm obviously checking the user always has permission to do anything before doing any actions.) Because of the `$base` it will always be someplace on my domain so worst case scenario is that they get a 404 (but that's there fault anyway because they messing with things(. My real question is: is there any thing malicious they can inject with that `return` variable? – Chaim Nov 27 '11 at 14:53
  • Perhaps ... if into the return variable an attacker can put line feeds ... bingo ! new http headers after Location. I am not sure there aren't web browsers flaws about http headers parsing. – Massimo Nov 27 '11 at 15:30
  • A friend of mine once used a $_GET variable in an include statement. Looking at the apache logs, I found out that the script could be used to gain admin command line access to our server. It was creepy. – Frank Nov 27 '11 at 15:57
  • @Frank so what? How is your story relevant to the question? – Your Common Sense Nov 27 '11 at 16:35
  • 1
    @Frank Oah yeah well I once used a GET variable to kill a man. But i don't see how an LFI vulnerability has anything to do with this. – rook Nov 27 '11 at 17:06
  • @Massimo perhaps you are thinking of http response splitting, and the header() function is no longer vulnerable to this. – rook Nov 27 '11 at 17:09
  • Stefan, dude. Don't you realize that a user can call whatever controller directly, without fooling with form at all? – Your Common Sense Nov 27 '11 at 18:42
  • Certainly, I know that anyone is able to call anything at any time. Trivial. But if a certain controller should be called after the form submit, I propose to ensure, that the controller gets called. Therefore the parameter should be validated. There are tons of protection options. My above mentioned 'Flow' is a conceptually very strong one. http://stackoverflow.com/questions/8002956/does-this-conform-to-a-certain-pattern – SteAp Nov 27 '11 at 18:43
  • you have to verify it at recieve time, not sending time. there is absolutely no point in your post. – Your Common Sense Nov 27 '11 at 18:47
  • So, if SO were written using your framework, there would be no question id in the question url - right? – Your Common Sense Nov 27 '11 at 19:25
  • Yes, right. To answer your next question: My simple approach isn't useful for a SO type site, since SO needs permanent and public visible URLs. Most apps I write are closed group apps. Nevertheless, the framework provides means of persistent Flows. – SteAp Nov 27 '11 at 19:29
  • @Col. Shrapnel My previous answer was too strict. Instead of a FlowID hashes generated from the next controller to be called, one could easily use a conventional URL as a vector pointing to a flow. Nevertheless, the Flow 'pattern' is quite universal. One can even pass complex arrays or serialized objects around. – SteAp Nov 27 '11 at 21:07
-3

I upvoted Stefan's answer.

I also have this to add. I wrote a nice class for building and parsing URLs. You could use it for validation, if you'd like.

See Url.php and UrlTest.php for usage.

https://github.com/homer6/altumo/tree/master/source/php/String

Hope that helps...

Homer6
  • 15,034
  • 11
  • 61
  • 81
  • I'm sure this could have been a lovely comment on Stefan's answer because it doesn't seem to answer my question. – Chaim Nov 27 '11 at 16:59
  • I can't see a reason why this answer got downvoted. I can't see a problem here. Thus: Upvote. – SteAp Nov 27 '11 at 18:34