Opened 6 years ago

Closed 4 years ago

#1484678 closed Bugs (fixed)

Method session_start with custom session handlers is not thread-safe.

Reported by: micha Owned by:
Priority: 5 Milestone: 0.2.2
Component: Core functionality Version: 0.1-rc2
Severity: normal Keywords: login redirect session periodical
Cc:

Description

There is a problem with the login, where after some logins and logouts, every new Login-Attempt kicked me back to the login screen. Without any mention of a problem in the logs.

The problem is, that the method session_start with the custom session handlers is not thread-safe. So it is possible that the logout and the periodical mail checking process occur at the same time.

The events occur in the following order:

  1. sess_read called by logout.
  2. sess_read called by periodical Mail-Check.
  3. sess_write (with $vars (temp|b:1)) called by logout.
  4. sess_write ($vars without temp) called by periodical mail check, overwrites sess_write from the step before.

Step 4 ("mail check") overwrites the Session-Parameters from Step 3 ("logout"). The concrete problem in that case is the temp-Parameter. Next login, session_start reads in the session parameter, where "temp" must be true, to start a new session. Otherwise when ("temp" == false), roundcube expects a valid session and tries to resume that session. (In index.php, $_SESSION\['temp'\] will be checked but fails.)

The only way a new login is possible, is to reset the cookies (restart IE and delete cookies in Firefox).

From my point of view, the php session methods (session_start, session_destroy, ...) that use the custom session handlers (sess_open, sess_close, sess_read, sess_write, ...) must be atomic. So when two session_start methods occur at the same time, they don't influence each other any more.

Change History (5)

comment:1 Changed 5 years ago by seansan

  • Milestone set to 0.1.5

Review in 0.1.5

comment:2 Changed 5 years ago by anonymous

  • Milestone 0.1.5 deleted

Milestone 0.1.5 deleted

comment:3 Changed 5 years ago by alec

  • Milestone set to 0.2-beta

comment:5 Changed 4 years ago by robin

  • Resolution set to fixed
  • Status changed from new to closed

Problem _mostly_ solved in [617b4f69].

If race conditions still occur, session variables will need to be read/saved individually, requiring changes to the database.

Note: See TracTickets for help on using tickets.