php / php-src

The PHP Interpreter
https://www.php.net
Other
37.93k stars 7.72k forks source link

Vulnerability due to insecure default values for session.cookie_secure and session.cookie_httponly #7913

Open etkaar opened 2 years ago

etkaar commented 2 years ago

Description

Dear colleagues,

it seems that the default values for the SECURE and HTTPONLY flags of cookies, especially for the PHP session cookie, (PHPSESSID) are not set to true. This opens a hidden vulnerability for serious XSS attacks.

Developers which use setcookie() and explicitly define $secure and $httponly might not be aware (they have done their work and feel safe), that they additionally have to explicitly set these values in session_set_cookie_params() to protect the PHPSESSID cookie from being stolen due to trivial XSS attacks:

session_set_cookie_params(['secure' => true, 'httponly' => true]);

I kindly ask you to confirm this issue and move to secure-per-default for this.

Kind Regards, etkaar

PHP Version

PHP 7-8

Operating System

No response

cmb69 commented 2 years ago

Indeed, session.cookie_secure and session.cookie_httponly default to off. I don't consider this to be a bug, though, especially since there is explicit documentation about securing session INI settings.

I'm not against changing the defaults for the next minor version, though.

etkaar commented 2 years ago

Thank you for your answer!

It might be not a bug, but I would consider defaulting to an unsafe value as a vulnerability, since there is simply no reason for such an unsafe-per-default scheme. In case developers want the PHPSESSID cookie – which is the primary target of XSS attacks – to be accessible via JavaScript (even if I cannot see any use case for that), they know how to enable that.

David263 commented 2 years ago

At what version has the default security option been set for PHPSESSID?

David263 commented 2 years ago

I tested session_set_cookie_params(['secure' => true, 'httponly' => true]); in PHP 7.4.13 and found that it fails to set PHPSESSID to 'secure'.

KapitanOczywisty commented 2 years ago

I'm not against changing the defaults for the next minor version, though.

Now ssl is free and easy to use, and there is no real reason to access PHPSESSID from JS, so both probably should be changed for On. Also default values are more than a decade old.

David263 commented 2 years ago

I'm not sure I agree, Kapitan. I'm not familiar with the JavaScript attack vector. But being able to copy the value of PHPSESSID is useful in JavaScript, so that JavaScript can similarly remember session data. But the actual contents of a session array must not be readable from JavaScript, since it is server-side data, regardless of the 'secure' option.

KapitanOczywisty commented 2 years ago

But being able to copy the value of PHPSESSID is useful in JavaScript, so that JavaScript can similarly remember session data.

This is very dangerous since attacker by injecting own JS code (e.g. compromised advert, extension or XSS) can get full access to user session. JS should be using sessionStorage or localStorage for that, and probably keep only low-value informations like dark mode etc. Anything sensitive should be kept on server-side.

But the actual contents of a session array must not be readable from JavaScript, since it is server-side data, regardless of the 'secure' option.

It never is.

etkaar commented 2 years ago

I'm not sure I agree, Kapitan. I'm not familiar with the JavaScript attack vector. But being able to copy the value of PHPSESSID is useful in JavaScript, so that JavaScript can similarly remember session data. [...]

I strongly do not agree. The PHPSESSID is a very sensitive value, it must not be readable via JavaScript per default.

David263 commented 2 years ago

Then how is JavaScript code to know that it is in the same session as the one the server thinks it's in? And how can an Ajax call propagate the session ID to further server code if it can't be read? Security is one thing, but the ability to do coherent Web operations is also important. Of course, in the final analysis, the trouble is that so much of Web programming is a kludge. HTTPS was an add-on, so it is still optional. And client-side code can never be secure until or unless the client OS and browser implement true security even from the computer owner, meaning Ring 0 security.

etkaar commented 2 years ago

Session information is transmitted to the server by the browser using cookies, which are sent within the HTTP headers.

JavaScript itself is usually not required or expected to interact with cookies, because it is a programming language which – in server-side-driven architectures (SSR) – only happens (is executed) client side, while the cookies are a client->server information.

A case where it is appropriate to let JavaScript know or even modify the value of a cookies is, for instance, a setting which has not necessarily to be reflected server-side. If you want to allow users to enable or disable darkmode without a request to the server being required, you can remove the httponly flag and let JavaScript handle all of that. Downside: The setting won't be reflected on all devices, as it is not transmitted to the server.

David263 commented 2 years ago

The whole problem in this Issue is reflected in your phrase, "you can remove the secure flag". In point of fact, PHP 7.4.13 does not even implement the secure flag. See my postings above for details. Security should be maximum by default. The idea of increasing the security of an insecure connection is a bad one.

I usually use Ajax when I need to extend browser JavaScript to do things only the server can do, like write to a file on the server (which can be a local server). For such functionality, security should be highest by default.

etkaar commented 2 years ago

That is why I opened this issue: Both the secure and httponly flags must be set per default for PHPSESSID, setcookie() and setrawcookie(). At this time, both are set to false unfortunately. In my previous post I accidentally referred to the secure flag, I – of course – in fact meant the httponly flag. It is this flag which prevents JavaScript from interfering with cookies.

KapitanOczywisty commented 2 years ago

In point of fact, PHP 7.4.13 does not even implement the secure flag.

These flags are available since PHP 5 and not being removed or deprecated in any way, the only discussion is about defaults. Make sure you have session.auto_start=0 (if setting flags runtime), then you use session_start after session_set_cookie_params and that you don't have existing PHPSESSID, since PHP will likely not send Set-cookie header if PHPSESSID exists.

David263 commented 2 years ago

etkaar, That is why I'm posting here. I completely agree with your Issue.

KapitanOczywisty commented 2 years ago

Both the secure and httponly flag must be set per default for [...] setcookie() and setrawcookie().

I wouldn't touch these ones at least not now, changing default for PHPSESSID would be more or less painless, but change in setcookie will break some websites, and as you mentioned in the first message, developers are aware of these options.

etkaar commented 2 years ago

Both the secure and httponly flag must be set per default for [...] setcookie() and setrawcookie().

[...] but change in setcookie will break some websites, and as you mentioned in the first message, developers are aware of these options.

Well, that is correct, yes. The defaults for setcookie() and setrawcookie() could be changed in a major release.

David263 commented 2 years ago

Kapitan, I would like to prevent session autostart, but your code doesn't work and Web searching doesn't find a way to do this at runtime, just in the PHP.INI file.

KapitanOczywisty commented 2 years ago

You could try to set php_value session.auto_start 0 in .htaccess, but it's best to edit .ini file. Some providers have ini settings accessible through control panel. Of course for testing you can use session_regenerate_id to resend PHPSESSID.

Also: https://www.php.net/manual/en/configuration.file.per-user.php

David263 commented 2 years ago

I put the autostart disabling into .htaccess and ran my test. The result was PHPSESSID cookie has HttpOnly: false, Secure: false. My code:

    session_set_cookie_params(['secure' => true, 'httponly' => true]);
    session_cache_limiter("nocache");
    session_cache_expire(SESSION_MINUTES);
    session_start();
etkaar commented 2 years ago

If you're using PHP 7.3 or higher, that is strange.

KapitanOczywisty commented 2 years ago

I put the autostart disabling into .htaccess and ran my test. The result was PHPSESSID cookie has HttpOnly: false, Secure: false.

Make sure changes in .htaccess are actually effective on your server, use print_r(ini_get_all('session', false));.

I've tested session_set_cookie_params on my server and it's working perfectly fine even on PHP 7.4. session_set_cookie_params in fact changes ini settings: https://3v4l.org/cWO3Z Problem is somewhere on your end, and this is not good place for troubleshooting - issue went off topic.

David263 commented 2 years ago

cookie_secure is true, auto-start is false.

KapitanOczywisty commented 2 years ago

Make sure to remove existing PHPSESSID, then after session_start you can verify what php is setting in set-cookie header: print_r(headers_list());.

cmb69 commented 2 years ago

and this is not good place for troubleshooting - issue went off topic.

That. See https://www.php.net/support about available support channels; and of course, there is also StackOverflow etc.

joostgrunwald commented 1 year ago

This has slacked a bit, but in my opinion setting the HTTPONLY and Secure flags should indeed be default behaviour. At the moment defaulting to a insecure setting has no good reason, and I often find web applications that leave this behaviour at the default setting without understanding that this could help to address potential serious risks.

KapitanOczywisty commented 1 year ago

This might require RFC, but please contact mailing list first https://wiki.php.net/rfc/howto

joostgrunwald commented 1 year ago

I did the first step, will wait for reaction

etkaar commented 1 year ago

I did the first step, will wait for reaction

Thanks!

David263 commented 1 year ago

Just as the Let's Encrypt initiative resulted in better security for the Web, so would setting these two cookie options by default, clearing them only for special purposes (maybe HTTPONLY for using JavaScript for testing, or SECURE for IoT applications that must run on insecure http connections).

iluuu1994 commented 10 months ago

@joostgrunwald Did you ever send out that e-mail? I cannot find it.

DaWe35 commented 6 months ago

Hi, my website just got exploited by XSS and the attacker had access to the session cookies because of the unsafe session cookie defaults. I'm really sad that this conversation died between off-topic comments, so I wrote down all my thoughts in a PR: https://github.com/php/php-src/pull/13720

etkaar commented 6 months ago

And I reported this vulnerability already years ago to the PHP team ☹️

xperia-query commented 4 months ago

PHP is language but PHP has built in server module. so PHP's default settings should be changed to safe settings.

I think a good language should be default setting simple and secure.

etkaar commented 7 hours ago

@cmb69 Hi! Any way to push this issue a little bit forward? At least for PHP 9 it should be no problem to change the default to secure, as insecurely configured cookies as default can be considered as unexpected behaviour.