4

I have an application which is structured in the following way for its signup page. After this signup, the user is directly granted access into the system - there is no email verification (as intended).

I'm interested in learning if there are attack vectors or other insecurities in how this workflow is constructed.

  1. ScriptA.php takes user input in a FORM
  2. When it is submitted, it is POSTed to ScriptA-verify.php
  3. This second script verifies the information POSTed to it
  4. If there is a problem, it POSTs the information back to ScriptA.php (so that the information is retained for the user)
  5. If there is no problem, it adds the data to the database, sets a SESSION['userid'] variable, etc and POSTs some hidden fields to landing.php
  6. In landing.php, we check if SESSION['userid'] is set
  7. If it is not, we throw the user to an error page
  8. If it is, we proceed with the application
AviD
  • 72,138
  • 22
  • 136
  • 218
Alan Beats
  • 141
  • 2
  • Can you give more details on step 4? I mean, the script on the server isn't going to send the POST request to its sibling on the same server directly. I guess its response is going to generate something that contains the data that was submitted and perhaps have an "onLoad" form submit that content back to the initial script (in which case the user's browser is going to make this POST again). – Bruno May 25 '11 at 15:06
  • Actually @bruno - it is doing that... it creates a whole set of fields for all the POST values, places them within and does a to POST it back to scriptA.php – Alan Beats May 25 '11 at 15:51

2 Answers2

5

It's difficult to give you a detailed answer as a lot will depend on the actual code you use to implement this process. The first and arguably most important thing to consider is input AND output validation on the data supplied by the user. There are plenty of good resources out there about that, http://www.owasp.org/ springs immediately to mind as the "go to" for that.

In terms of your actual approach, I would suggest that rather than having ScriptA.php POST to ScriptA-verify.php you should POST to ScriptA.php and handle the verification in there. By all means accomplish this by performing an "include" of ScriptA-verify.php but what this means is that you do not have the added complexity and consequently risk, of posting data back to ScriptA.php if ScriptA-verify.php returns a problem. The original form data will be available to you from your sanitised variables derived from $_POST (do not use $_POST directly, see above wrt validation).

The $_SESSION superglobal is "pretty secure" but there's a decent post here which you can find out a bit more from: Altering a $_SESSION variable in PHP via XSS?. There are others on Security StackExchange too.

I don't get a warm, fuzzy feeling that your check of $_SESSION['userid'] is going to be particularly bombproof though a lot of that will depend on knowing other specifics of the process you are developing. For example, ability to session hijack if session is conducted over HTTP not HTTPS, PHP configuration settings on your server, etc.

Another question I would ask is what checks are in the other pages that this login will protect otherwise you are potentially vulnerable to such things as Insecure Direct Object Reference. In short, don't assume that people will always come through landing.php to get to "secured-page1.php". If there's nothing in secured-page1.php to ensure a valid session all your efforts are in vain.

I also don't see anything which is going to protect your users from CSRF attack. Again it depends what the application actually does but if the only check before loading a page is for a valid session then it would be trivial to force a user's browser to request specific pages such as password resets or such like. Again, the OWASP site can help you here.

This is something of a quick taster, brain dump answer. If you can give more specific details of the code behind your app and possibly your experience level it may be possible to give you more low level and thorough advice.

Community
  • 1
Marc Wickenden
  • 671
  • 3
  • 6
  • (1) I understand keeping it all in the same file and POSTing it to itself is neater - but whats the difference between POSTing it to itself and POSTing to another file? (2) I chekced the other post you mentioned - but it does not look like that's a way of hacking into sessions variable, so I should be fine. (3) can you help me understand what you mean by Insecure Direct Object Reference? What would you like to know more about? I'm a independent consultant, working in the industry for a couple of years - medium level knowledge of php though – Alan Beats May 25 '11 at 16:06
  • 1) By POSTing it to the same file you are removing the need for step 4 where you POST back to your form internally. Bear in mind that onLoad executes javascript which some people disable. I wouldn't rely on it for such a core functionality when it's unnecessary in the first place. 2) Looks OK 3) Maybe it wasn't clear but if I know the location of a page on your site I can request it directly (type it in the address bar for example). Don't assume because the only links to the page are from an authenticated page that I can't get to it. That's security through obscurity. Does that help? – Marc Wickenden May 26 '11 at 08:20
  • @Alan Beats - I've knocked up a very quick and dirty example of a php script which will handle the input, perform some validation and re-render the form safely with the user supplied input if there's an error with the script. You can get the source for the script at http://pastie.org/1975182. Very simple, you must enter wicky in the form field. – Marc Wickenden May 26 '11 at 08:23
  • it sounds like you're referring more to ["A8: Failure to Restrict URL Access"](https://www.owasp.org/index.php/Top_10_2010-A8). The other one - ["Insecure Direct Object References"](https://www.owasp.org/index.php/Top_10_2010-A4) - refers more to authorizing by record key (e.g. account number). – AviD Jun 05 '11 at 09:34
  • @AviD Yep, you're right. I was, thank you for the correction! – Marc Wickenden Jun 29 '11 at 08:36
2

Here are a few points you should consider and take care of, in addition to @wicky's good answer:

  • Input validation, each time the data hits the server. (Do not rely on previous validations).
  • Output encoding - any data you output must be properly encoded according to the current context (e.g. dont use HTML encoding if you're writing to an attribute value).
  • Make sure to use $_POST , and not $_REQUEST - variables can be written directly to the URL and retrieved from the GET, even though you expect them to be in the POST.
  • There are some fields which should NOT be sent back to the user, e.g. passwords. Make the user fill the password in again, dont repost it.
  • Log each failure (but not the passwords).
  • Don't roll your own session management, use the existing one provided by the framework. There are too many things to consider there, and you'll probably get some of them wrong - session id generation, validation, expiration, etc....

Overall I like @wicky's suggestion of validating in the same page, instead of bouncing back and forth, and encouraging attackers to find a weak spot in all that re-posting. Minimize attack surface, and all that...

AviD
  • 72,138
  • 22
  • 136
  • 218