Use httpOnly cookies

[Christopher Kramer]

Originally reported on Google Code with ID 174

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

Reported by crazy4chrissi on 2013-02-09 21:53:24

[Christopher Kramer]

(This has first been mentioned as a comment in issue #170.)

Reported by crazy4chrissi on 2013-02-09 21:54:36

[dreadnaut]

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.

Reported by dreadnaut on 2013-02-10 14:24:10 - Status changed: Accepted - Labels added: Security

[Christopher Kramer]

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.

Reported by crazy4chrissi on 2013-02-10 16:14:18

[Christopher Kramer]

Typo: "If we still want to support PHP 5.1" (not 5.2)

Reported by crazy4chrissi on 2013-02-10 16:15:21

[dreadnaut]

> 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.

Reported by dreadnaut on 2013-02-11 13:25:34

[Christopher Kramer]

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?

Reported by crazy4chrissi on 2013-02-15 15:03:52

[dreadnaut]

Sure, a patch against r338 is attached.

Reported by dreadnaut on 2013-02-15 23:39:06

<hr> * Attachment: httponly-r338.diff

[Christopher Kramer]

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.

Reported by crazy4chrissi on 2013-02-16 11:32:52 - Status changed: Started

[dreadnaut]

This issue was closed by revision r339.

Reported by dreadnaut on 2013-02-16 11:47:08 - Status changed: Fixed

[dreadnaut]

> 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.

Reported by dreadnaut on 2013-02-16 11:47:59

[Christopher Kramer]

> 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.

Reported by crazy4chrissi on 2013-02-16 13:19:17