search

sapioit / phpliteadmin

Automatically exported from code.google.com/p/phpliteadmin
1 stars 0 forks source link

Use httpOnly cookies #174

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
I would propose to use httpOnly cookies. It is a great help against XSS/cookie 
attacks (as it makes it impossible in modern browsers to steal cookies from 
JavaScript through XSS).

Only thing is PHP 5.2 would be required to use setcookie(), or we'd need to set 
cookies manually using header().

We currently require PHP 5.1.0. But as PHP 5.1 (and 5.2) is not maintained any 
longer for some time, I would have no problem to raise the required version.
Of course we could also use a fallback if we want to. But I also think it's 
good to drop support for old versions to force server admins to update.

I'd also propose use of httpOnly cookies for session cookies.
I.e. iniset this setting:
session.cookie_httponly = On

Original issue reported on code.google.com by crazy4ch...@gmail.com on 9 Feb 2013 at 9:53

GoogleCodeExporter commented 9 years ago 
(This has first been mentioned as a comment in issue #170.)

Original comment by crazy4ch...@gmail.com on 9 Feb 2013 at 9:54

GoogleCodeExporter commented 9 years ago 
httpOnly is always good to have, and 5.2 also sounds like a sensible minimum 
requirement (I've seen a few large and slow organisations switch to 5.3 
recently).

We might however wait a bit for setCookie and for now just add the iniset line 
for session cookies. In the meanwhile, we make sure that our output is properly 
sanitized and no rouge javascript can end up inside our pages.

Original comment by dreadnaut on 10 Feb 2013 at 2:24

GoogleCodeExporter commented 9 years ago 
Hmm. Does it make sense to use httpOnly cookies for sessions but not for the 
remember-me cookies?
If there is an XSS vulnerability, the remember-me cookies (salt + hashed pw) 
would allow the attacker access for a longer time than the short-living session 
cookie would. So using httpOnly for session cookies will not hurt, but also 
won't improve anything as long as we don't use httpOnly for the other cookies.

If we still want to support php 5.2, we could just do:
if (version_compare(PHP_VERSION, '5.2.0') >= 0) {
//setcookie with httpOnly parameter
} else
{
//setcookie without httpOnly parameter
}

"In the meanwhile, we make sure that our output is properly sanitized and no 
rouge javascript can end up inside our pages."

Yeah well. We should make sure this cannot happen anyway because httpOnly 
doesn't save us against all XSS attacks. It only makes sure cookies cannot be 
accessed in modern browsers, but there are various other XSS attacks one could 
think of (e.g. some JS that sends some form to modify the db).

But it is a lot easier to use httpOnly than to make 100% sure all output is 
sanitized properly. I mean, a lot of effort has gone into this recently and I'd 
say phpLiteAdmin makes a good job here now (and did a very poor one some 
version numbers before). But I could not say I'm 100% sure that there is no XSS 
attack possible.

Original comment by crazy4ch...@gmail.com on 10 Feb 2013 at 4:14

GoogleCodeExporter commented 9 years ago 
Typo: "If we still want to support PHP 5.1" (not 5.2)

Original comment by crazy4ch...@gmail.com on 10 Feb 2013 at 4:15

GoogleCodeExporter commented 9 years ago 
> But I could not say I'm 100% sure that there is no XSS attack possible.

If only all output went through a small set of functions -sigh- :) :) :)

No, it doesn't make that much sense after all. After searching around for stats 
about php versions (they are rare) I think that we can go ahead and ask at 
least for 5.2.x.

Original comment by dreadnaut on 11 Feb 2013 at 1:25

GoogleCodeExporter commented 9 years ago 
You can find good and recent (daily!) stats about PHP versions here:
http://w3techs.com/technologies/details/pl-php/all/all
(They analyse the top 1 million alexa sites.)

For PHP 5.1, they say that 2.4% of PHP-servers use PHP 5.1:
http://w3techs.com/technologies/details/pl-php/5.1/all

I agree, it's totally okay to ask for PHP 5.2 (which still makes 37.2% of all 
php-servers, i.e. we should not drop support for 5.2 too soon.)

Do you want to implement it as you are mainly involved in the 
Authorisation-class?

Original comment by crazy4ch...@gmail.com on 15 Feb 2013 at 3:03

GoogleCodeExporter commented 9 years ago 
Sure, a patch against r338 is attached.

Original comment by dreadnaut on 15 Feb 2013 at 11:39

Attachments:

GoogleCodeExporter commented 9 years ago 
Thanks! Looks good. I am not sure whether it makes sense to use httpOnly while 
unsetting the cookie, but it definitely won't hurt.

Please commit it.

We should keep in mind that we now expect PHP 5.2 at least. Therefore we should 
mention this in the release notes and should update the website after release.

Original comment by crazy4ch...@gmail.com on 16 Feb 2013 at 11:32

GoogleCodeExporter commented 9 years ago 
This issue was closed by revision r339.

Original comment by dreadnaut on 16 Feb 2013 at 11:47

GoogleCodeExporter commented 9 years ago 
> I am not sure whether it makes sense to use httpOnly while unsetting the 
cookie,
> but it definitely won't hurt.

Same here, I added mainly because the php documentation states that "Cookies 
must be deleted with the same parameters as they were set with."

Committed as r339.

Original comment by dreadnaut on 16 Feb 2013 at 11:47

GoogleCodeExporter commented 9 years ago 
> Same here, I added mainly because the php documentation states that
> "Cookies must be deleted with the same parameters as they were set with."

Ah, okay. Thanks for making me aware of this.
And thanks for the fix.

Original comment by crazy4ch...@gmail.com on 16 Feb 2013 at 1:19