Yep, Another login script

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: Yep, Another login script

Post by Benjamin »

I worked on a social networking site that had to pass the session id around on every single page because their research indicated that most of their users didn't have cookies enabled. Many of the pages had all sorts of links and forms, so the session id was going to be sent one way or another.
In particular, the convention has been established that the GET and HEAD methods SHOULD NOT have the significance of taking an action other than retrieval.
As I said earlier, do not perform any actions with data that comes from a GET request. You don't want google to come in and delete all your records.
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: Yep, Another login script

Post by kaisellgren »

astions wrote:I worked on a social networking site that had to pass the session id around on every single page because their research indicated that most of their users didn't have cookies enabled. Many of the pages had all sorts of links and forms, so the session id was going to be sent one way or another.

Code: Select all

$req = array_merge($_GET,$_POST);
echo $req['session_id'];
That's safer in general use.

Or if you also need the id from the cookies, then use $_REQUEST, but use $_GET and $_POST for all other information.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: Yep, Another login script

Post by Benjamin »

Request worked fine and didn't cause any security issues.
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: Yep, Another login script

Post by kaisellgren »

astions wrote: didn't cause any security issues.
"Twitter was secure until last week" 8)
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: Yep, Another login script

Post by Benjamin »

Well there are arguments that go both ways. I wouldn't recommend using $_REQUEST unless you are experienced and know what you are doing. Using $_REQUEST isn't going to automatically make your application inherently insecure, but it could if used incorrectly.
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: Yep, Another login script

Post by kaisellgren »

astions wrote:Well there are arguments that go both ways. I wouldn't recommend using $_REQUEST unless you are experienced and know what you are doing. Using $_REQUEST isn't going to automatically make your application inherently insecure, but it could if used incorrectly.
Conclusively. $_REQUEST is a feature that PHP offers, using it is okay if you use it for what it is for. In big applications, having $_REQUEST to replace $_GET, $_POST or $_COOKIES is not the best approach to take if you care about security.

The same thing can be said about many other features or functions. For instance, using addslashes() can be perfectly healthy, but the thing is, it is usually implemented wrong or used for wrong purposes.
gregor171
Forum Newbie
Posts: 22
Joined: Thu Apr 16, 2009 5:09 pm
Location: Ljubljana, Slovenia

Re: Yep, Another login script

Post by gregor171 »

It might help to do some base reading:
http://phpsec.org/library/
http://cwe.mitre.org/top25/
temidayo
Forum Contributor
Posts: 109
Joined: Fri May 23, 2008 6:17 am
Location: Nigeria

Re: Yep, Another login script

Post by temidayo »

astions wrote:Passing the session id around is a great reason to use $_REQUEST.
I have seen it in internet marketing script that passes affiliate ids.
That could include cookies.
kaisellgren wrote:Conclusively. $_REQUEST is a feature that PHP offers, using it is okay if you use it for what it is for. In big applications, having $_REQUEST to replace $_GET, $_POST or $_COOKIES is not the best approach to take if you care about security.
The summary as I see it, is that it is the $_COOKIE in $_REQUEST that is the problem or at least the bone of contention here.
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: Yep, Another login script

Post by kaisellgren »

I just experienced an issue with this. A client (well, actually my friend's client) ran web hosting business where he allowed free sub-domain hosting. The web hosting admin control panel system, which manages the website, was using $_REQUEST all over the place (I was evaluating the code) and it led to a vulnerability. Since I knew the exact code, it was very easy for me to exploit the application. Basically, the vulnerable code was similar to this:

Code: Select all

if (isset($_REQUEST['deleteUser']))
{
// Checks for tokens (anti-CSRF) and deletes the user
}
I created a hosting account, made a PHP script to output set-cookie and I set a property "deleteUser" to be 1. I asked the site administrator to visit my "site" and sooner than I thought the user with ID 1 (the admin) was deleted. The server logs show this:

Code: Select all

GET ******?deleteUser=1396******&Stoken=DQAAAIIAAABvsQ562J4x7CTIsHct4_lLHPtGAY_Y8ezcWO3DtfPgbgqltk23dgJC
      Host ******
      User-Agent Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
      Accepttext/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
      Accept-Languageen-us;q=0.5,en;q=0.3
      Accept-Encodinggzip,deflate
      Accept-CharsetISO-8859-1,utf-8;q=0.7,*;q=0.7
      Keep-Alive300
      Connectionkeep-alive
      Referer ******
[color=#0080FF]      Cookie ****** deleteUser=1 *******[/color]
 
Sorry for the messy text, his logging system really sucks.. (not talking about web server logs)

The site was protected against typical CSRF attacks, but a delayed CSRF attack was possible, because the site relied on $_REQUEST. The site admin did indeed remove a user, but the value from the cookies overrode the intended user ID.

By the way, PHP 5.3 does not include $_COOKIES in $_REQUEST thanks to a few security experts sharing their opinions. So, if you want to get data from all three channels, better use array_merge().
Post Reply