Cookies should be set as 'secure' over SSL

[mkj]

Even if a website is set up using SSL, an active attacker can steal cookies unless the cookies have been set 'secure' - see ​http://fscked.org/blog/fully-automated-active-https-cookie-hijacking

Roundcube doesn't set cookies as secure. It looks like the only place that needs changing is the second setcookie() in
session.inc

Something like

setcookie(session_name(), $random, $lifetime, $cookie['path'], $cookie['domain'],  $_SERVER["HTTPS"]);

might be the way to go? (I haven't used PHP much so am guessing from the docs).

[robin]

Index: program/include/session.inc
===================================================================
--- program/include/session.inc (revision 1760)
+++ program/include/session.inc (working copy)
@@ -184,7 +184,8 @@
   $lifetime = $cookie['lifetime'] ? time() + $cookie['lifetime'] : 0;

   setcookie(session_name(), '', time() - 3600);
-  setcookie(session_name(), $random, $lifetime, $cookie['path'], $cookie['domain']);
+  setcookie(session_name(), $random, $lifetime, $cookie['path'], $cookie['domain'],
+                       $_SERVER['HTTPS'] && ($_SERVER['HTTPS']!='off'));

   return true;
 }

I have no test-RC running over https so cannot test that. Over http this doesn't have any impact.

[mkj]

I've tested that patch here (with roundcube 0.1.1 though) and it sets the logged-in sessionid cookie as secure, but doesn't set the initial pre-login session cookie as secure. I'm not sure if that really matters though? Does the pre-login session cookie get used for anything?

session_set_cookie_params() could be used to setup the initial cookie as secure I think.

[tensor]

I guess the prelogin cookie should also be secured.

[robin]

Pre-login does not need to be 'secured'. Session cookie security fixed in [d0b973cf].