bitbucket-import commented 13 years ago

=== Issue === When using the sessions library and the following conditions exist:

$config['sess_use_database'] = true;
The period specified in $config['sess_time_to_update'] has expired

It is possible to inadvertently trigger destruction of the session by performing two requests at the same time. An example of a scenario that easily triggers this error is a page which initiates two or more AJAX requests at the same time.

=== Expected Behavior === The CodeIgniter sessions library should not inadvertantly destroy sessions.

=== Cause of Issue === This is what I believe to be occurring.

Two requests are made at the same time, causing them to submit the same cookie data.

When the server receives the request, the session library looks for the session matching the ID submitted on the cookie. (Session.php:192)

The first request finds the session record and updates it since sess_time_to_update has expired. (Session.php:377)

The second request still has the old session id, and fails to find the newly updated database record. This then triggers a sess_destroy. (Session.php:212)

=== To Replicate ===

In config.php: {{{ // Reduce the amount of time we have to wait to see the bug to 5 seconds. $config['sess_time_to_update'] = 5; }}}

Create controller session_error_demo.php: {{{ <?php class Session_error_demo extends Controller { function initialize () { $this->session->set_userdata('session_alive', 'Hi world!'); echo 'Session initialized.'; }

function test() {
    if ($this->session->userdata('session_alive')) {
        echo 'Session is alive.';
    } else {
        echo 'Session is dead.';
    }
}

function reset() {
    $this->session->sess_destroy();
    echo 'Session reset.';
}

} ?> }}}

Perform the following tasks:

Call reset() to ensure we start on a clean slate.

Call initialize() to seed a session value.

Wait 5 seconds so that the session will be forced to update on next request.

Call test() twice in rapid succession, so that both requests will be sent before either is fully processed. I used jQuery & Firebug to submit these requests using $.get().

View the responses. The first will report that the session is alive, but the second and all subsequent requests will report that it is dead.

=== Versions Affected === This issue exists back in my version on 1.7.2. I haven't verified that this still exists in 2.0.2, but it appears that the buggy code remains intact.

sergio-ascanio commented 13 years ago

I've a similar Issue, my app keep alive making ajax requent periodically, when I open another tab from my web app sometimes I lost my session

PHP-Point-Of-Sale commented 13 years ago

I have had this issue too, very annoying and hard to explain to clients. It even happens when sess_time_to_update is set to PHP_MAX_INT (I am not waiting that long either for it to happen)

dordal commented 13 years ago

I have a workaround for this based on inspiration from a CI Forum discussion; you can basically disable the session updates when AJAX calls are made. This code should go in /application/libraries/MY_Session.php :

<?php
class MY_Session extends CI_Session {

    /**
     * Update an existing session
     *
     * @access    public
     * @return    void
    */
    function sess_update() {
       // skip the session update if this is an AJAX call! This is a bug in CI; see:
       // https://github.com/EllisLab/CodeIgniter/issues/154
       // http://codeigniter.com/forums/viewthread/102456/P15
       if ( !($this->CI->input->is_ajax_request()) ) {
           parent::sess_update();
       }
    }
}

sergio-ascanio commented 13 years ago

thanks for share your fix dordal I will try it

ghost commented 12 years ago

Am still getting the same problem with 2.0.3

samxli commented 12 years ago

What if the web app is mostly made with JS and ajax? No session will ever be updated and the user will just timeout when the expiration has arrived?

sergio-ascanio commented 12 years ago

I use the following code to keep the session alive:

    function sess_update() {
            // skip the session update if this is an AJAX call! This is a bug in CI; see:
            // https://github.com/EllisLab/CodeIgniter/issues/154
            // http://codeigniter.com/forums/viewthread/102456/P15
            if ( !(isset($_SERVER['HTTP_X_REQUESTED_WITH']) && strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) == 'xmlhttprequest') || ( stripos($_SERVER['REQUEST_URI'], 'live/request') != 0 ) ) { 
                    parent::sess_update();
            }
    }

I only allow the request made by the controller that updates the app ( http://localhost/app/live/request ), every 5 minutes the session is updated by an ajax request, is an ugly hack but works fine so far.

yorickpeterse commented 12 years ago

Issue is still present in CI >= 2.0, the solution proposed by @dordal, while hacky, does the trick.

kochb commented 12 years ago

As the original reporter, it should be noted that this is still a fundamental flaw with the way session updates work.

I personally abandoned using database sessions altogether. @dordal has a good workaround, but even with that the bug can be triggered via means other than AJAX requests.

samxli commented 12 years ago

@kochb how are you storing your sessions now?

kochb commented 12 years ago

I actually don't even use CodeIgniter anymore and switched to another framework. I got fed up with CodeIgniter since there were too many issues getting left unaddressed, and it was harming quality and productivity.

In terms of actual solutions:

As a temporary fix, I turned off database sessions. Note that it will cause the session data to be stored in a cookie, so you need to determine whether that is acceptable to you. (See Saving Session Data to a Database, http://codeigniter.com/user_guide/libraries/sessions.html).

$config['sess_use_database'] = false;

There's also some drop in replacement libraries out there too that I used at various points that may be worth investigating. Haven't used them in a while, so I don't know what their status is.

Otherwise: If neither of those works, @dordal has the best workaround I've seen. You will still be vulnerable to lost sessions, but it will be much less of a problem.

ciribob commented 12 years ago

What is the best replacement library? The site is complete so i'd ideally just like a drop in replacement with the same functions for flashdata and retrieving data as the codeigniter library. I too have run into this issue so currently database sessions are turned off...

samxli commented 12 years ago

@kochb are you using database sessions with that new framework? If so, care to share what framework you are now using?

Dentxinho commented 12 years ago

I've been using this solution in a production system since 2009 (1.7.2 and 2.0.3 since Sep 2011) without any problems.

For those that have noticed, the thread is from Aug 2010, but this was not first posted there. This approach introduced a config parameter to control the generation of a new session_id.

I can't imagine that the best solution is not to update sessions from ajax requests. What about sites built on ExtJS?

kochb commented 12 years ago

Don't switch your framework over the session library. I'm using CakePHP right now; I liked the amount of workload it takes off you by automatically creating the CRUD for your database, its well organized and thorough class structure, and the fast response time when you do report issues. I've mostly trusted Cake's native session library to work and it hasn't given me problems yet.

If you're sticking with CodeIgniter and security is an issue, use the workaround @dordal gave or find a good drop in replacement. I was simply noting that the workaround does not address the fundamental problem; nonetheless the likelihood of triggering that problem drops drastically.

philsturgeon commented 12 years ago

The PyroCMS gang are stuck on this too:

https://github.com/pyrocms/pyrocms/issues/1049

This is a CI bug thats been around for ages that I know nothing about but really want to see nailed.

Dentxinho commented 12 years ago

@philsturgeon did you saw my post?

philsturgeon commented 12 years ago

So not updating the session id on AJAX is a known fix for this but is it secure?

If you leave something on a page it can keep making ajax calls forever and not expire, until somebody tries loading the next page.

narfbg commented 12 years ago

It would lower the security level a bit, but that's the problem with cookies - the more secure your implementation is, the more you're limiting your own options. Let's see what are the other "standard" techniques for securing cookies that we can use:

using HTTPS-only cookies; definately raises the bar, but it's not always possible
IP address and user agent string matching; should be enough in 99 out of 100 cases, but are optional and not convenient in some cases
using "httponly" cookies; only available with PHP >= 5.2 and can't be used with AJAX requests (that kind of the whole point)
enforcing use of non-empty cookie_domain and cookie_path settings; would help increase the possibility of a cookie being stolen, but again - not always really usable

... pretty much nothing that we can always rely on.

What we can do is make session ID regeneration during AJAX requests optional (and enabled by default), so developers that have this issue can disabled it. Probably should've thought of that earlier - I'll update the pull request, but I want to know your thoughts on another idea first ...

Encapsulate "tickets" inside our cookies, ala Kerberos. In order to do that, we need to have an randomly generated token inside the cookie and a list of such tokens + timestamps stored in the database (or whatever server-side storage we are using). Each of those tokens must only be valid for a specified range of time after sess_time_to_update. Here's how it works:

each time that the session ID is re-generated, a new token is generated as well and put inside the encrypted cookie data; any old token is invalidated
when sess_time_to_update expires, if we have an AJAX request - we still only update last_activity and don't change the session ID, but:
- we check if the token inside the cookie is inside our server-side tokens list
- delete any server-side tokens that have expired, although we might leave that to the garbage collector

If the token is found in our list and is inside the allowed timespan - generate a new one, insert it into our list (without deleting the old one) and send it back in the cookie. And here's why we don't delete the old token:

new AJAX requests will send the newly generated one back to us
any AJAX request received that was initialized by the browser before or while we were creating the new token will still send us the old one and is again checked against it's timestamp for validity

... and of course - if we receive an invalid token, we'll destroy the session.

This might as well be used with non-AJAX requests, but it might be a little too much.

What do you think?

ciribob commented 12 years ago

@narfbg I've patched the session to never regenerate the session ID (we also only use HTTPS so cookie stealing should be harder?) but this is a much better solution! Our site is AJAX heavy so the session disappearing bug happened frequently.

juanchorossi commented 12 years ago

Hi there, Ive updated to Phil's last version and I still have the problem. Thanks in advance

philsturgeon commented 12 years ago

Have you updated the CI session database since 2.0.3?

Emailing from my iPhone 4S like a BOSS!

On 4 Feb 2012, at 21:16, Juancho Rossireply@reply.github.com wrote:

Hi there, Ive updated to Phil's last version and I still have the problem. Thanks in advance

Reply to this email directly or view it on GitHub: https://github.com/EllisLab/CodeIgniter/issues/154#issuecomment-3815719

juanchorossi commented 12 years ago

Hi Phil, thanks for your reply. This its my ci_sessions structure: session_id, ip_address, user_agent, last_activity (int (10)), user_data (text). Is this ok?

Just to give a hint what could be the problem: My app prints some listings, and saves the current height of them. When scrolling down the browser, it requests more listings vía Ajax, saving again the current height of all the items. The first time its ok, but the next request its using the first values I've saved.

Ive solved by generating a session value userdata('listing_heightpage'.$page_number), not having to override its value.

Thanks in advance

Dentxinho commented 12 years ago

@juanchorossi Does the user_agent field have 120 length?

juanchorossi commented 12 years ago

Thanks!

Dentxinho commented 12 years ago

@juanchorossi, no no, it is right...

wallynm commented 12 years ago

Form who that don't know, Extjs it's a framework that works with basically ajax calls... Every page, every action of the user get all the data and js pages trought ajax cals, so like @Dentxinho said, jsut do not update the session id could be real problem for some people...

@Dentxinho i will try your two examples, thanks by posting them, that what i was lookking for!

philsturgeon commented 12 years ago

120 is ok, the PHP will truncate the string to that length.

Emailing from my iPhone 4S like a BOSS!

On 5 Feb 2012, at 05:32, Juancho Rossireply@reply.github.com wrote:

Yep, should I make it longer? | user_agent | varchar(120) | YES | | NULL | |

Thanks!

Reply to this email directly or view it on GitHub: https://github.com/EllisLab/CodeIgniter/issues/154#issuecomment-3817614

512uk commented 12 years ago

Correct me if I'm wrong (and I'm sorry if I'm barking up the wrong tree), but I don't think the issue is fixed. I have a pretty complex CI app that I've upgraded to 3.0-dev with the latest code from the develop branch, and the app utilises a fair amount of AJAX. A particular example is a Google Map which continually updates with locations every 10 seconds by making a request for JSON to the CI app.

My previous way of addressing the issue before @nafbg's pull request was to extend CI_Session and omit calling parent::sess_update() on an AJAX request, but I see you addressed this by always updating the last_activity on the session under AJAX requests. However, it seems as though last_activity is only sometimes being called, and therefore the session ID is being refreshed eventually.

Sorry I don't have too much more information, hopefully I will shortly. At the moment I'm just watching all XHR/AJAX requests in Chrome waiting for a timeout, as well as monitoring the database sessions to see if last_activity keeps updating, but sometimes it can get 5-10 minutes behind before refreshing. I know a minute ago before I started watching XHR activity the session did indeed time out.

Am I missing anything here?

Dentxinho commented 12 years ago

I don't see any way of your session ID to be changed, as session_update() dont change it under AJAX requests

512uk commented 12 years ago

Hey, I think it might have just been a blip or me being a noob. I've been watching AJAX requests now for 30 mins or more and haven't noticed it happen again. I'll keep an eye on the issue :) Sorry guys.

wallynm commented 12 years ago

At real, for me, this bug isn't solved too... But we have some nice workarounds here... :S As i said before, you could make like me @512uk... I have a app that every action use ajax calls, to get new pages, new data and even more... So, you will need to do not let the CI update the id_session, following this example http://codeigniter.com/forums/viewthread/102456/#523991 as @Dentxinho posted before, you will get a nice result... On my app i made some changes, insetead of just don't update the session id validating if it is a ajax request, i update their data as last_activity, but i keep the session_id as the same, so the CI will not logout you, and every time the the CI page get refreshed, it update the id_session as default behavior... As you said, isn't fixed, but is better than nothing!

narfbg commented 12 years ago

@512uk There's a sess_time_to_update configuration setting which sets the time needed to pass before the session table is updated. It's default value is 300 (in seconds, that's 5 minutes) - that explains why you need to wait a few minutes before you can see last_activity being changed.

Your issue is most likely caused by something that prevents AJAX requests to be detected as such. They are detected by checking if a X-Requested-With HTTP header exists and if so - it's value must be XMLHttpRequest. But there's one problem with that - browsers don't automatically detect if you're making an AJAX call and send it. It's just a custom header that is widely used and therefore - it's not guaranteed to always exist. This could be due to any of the following:

if you're using a JavaScript framework, it might not utilize it
if you're writing raw JavaScript - you need to put it in there manually
if you're behind a caching proxy, it might simply drop it (it's good to use separate URLs to access for regular and AJAX requests in order to avoid this)
it could be lost due to a browser bug (although, if you're using Chrome - that's most likely not the case)
some HTTP servers might not allow custom headers to be seen by PHP

... so there's no silver bullet for this one. I'll make another pull request that might help in case the header value is written e.g. in all lower case, but again - that's just one possible issue.

Areson commented 12 years ago

I know this issue is closed, but I've run across this same issue recently in a CodeIgniter app that I was working on. I didn't want to disabled sessions being regenerated from AJAX requests as the application I am working on functions as a "single page" application, using AJAX requests to fetch new content.

Some of the content I fetch also causes several AJAX requests (three or four) to be launched simultaneously. This resulted in one of them causing the session to regenerate which in turn would result in the remaining requests failing. In the case of the application this would result in the user being logged out, which they should not have been.

This is the approach I took and it seems to work well. Rather than require a user have just one session, a user is allowed to have up to N sessions. One of those sessions is the primary session, and should be the one that was most recently created. The rest are older sessions that are kept around in order to validate requests that came in with older sessions due to closely timed AJAX requests.

Each session has two "timeout" values associated with it. The first is the normal timeout that we would use to generate a new session one an existing on qualified for it. The second one is used to determine when a session should no longer be allowed to be used to validate a user.

One additional rule just for clarification: Once a session has been used to generate a new session, it can no longer be used to generate another. This effectively allows only the newest session to "spawn" a new session when the time comes.

The general idea is to keep a few older sessions around just long enough to prevent AJAX requests with older session data from failing. Here is an example to illustrate this:

We set our regeneration time frame (sess_time_to_update?) to be 60 seconds. We set our second timeout value to be 10 seconds. Now let's say we send two AJAX requests at about the same time. The first causes our current session to expire under the rules for the first time frame, so a new session is created and passed back to the browser. The second AJAX request doesn't have the session identifier, so it would normally fail. However, because we provide a 10 second window on the old session id, the second requests will succeed as long the old session is validated with the server within that window.

Depending on how complex your needs are, you could allow several old session to persist. As long as we only allow the most recent session to be used in creating new ones, we prevent someone from stealing an older session in order to take over a valid user's session, as the older session id will expire with the time frame we set without creating a new session.

dazld commented 12 years ago

I'm still having issues with this too - very worried that this appears to be a closed bug, when it's still being reported as a problem.. don't want to have to use a different framework!

Areson commented 12 years ago

@dazld I hear you. I'm going to try and make a pull request based on my comment above if I get the time.

svezina commented 12 years ago

Thanks Areson. Testing your pull request right now and it works like a charm. As far as I'm concerned, your pull request is going live in my stuff.

aphofstede commented 12 years ago

I see a "prevent.." semaphore in Areson's pull request; but isn't this solved easier by wrapping the read/write/update calls in Session::__construct() in a transaction? Edit: Ah, most of you are using myisam?

patricksavalle commented 12 years ago

We have had problems with the session-management in CI for a time. It looked to me like concurrency problems, for instance lots of (possible overlapping) AJAX requests from the same session, for instance when scrolling though a Datatables table.

If you look at the session code, it has a design flaw, in the session.php constructor:

    if ( ! $this->sess_read())
    {
        $this->sess_create();
    }
    else
    {
        $this->sess_update();
    }

It should be either a critical section or a real transaction. My solution was to use InnoDB tables for the session (we needed our session-table to be InnoDb for others purposes too, besides they perform -much- better in our tests) and put a transaction around it:

    if ($this->sess_use_database === TRUE)
    {
        $this->CI->db->trans_start();
    }

    if ( ! $this->sess_read())
    {
        $this->sess_create();
    }
    else
    {
        $this->sess_update();
    }

    if ($this->sess_use_database === TRUE)
    {
        $this->CI->db->trans_complete();
    }

It seems to work. Nobody in our team reports the unexpected logouts anymore.

CodeBrauer commented 10 years ago

have also a similar problem:

http://stackoverflow.com/questions/20654530/codeigniter-session-flashdata-kills-session-on-set-flashdata

harpreetsb commented 9 years ago

tried evrything above, Nothing seems to work. Any solution. My login is with ajax, so i see my session bein set in session response but not when page refreshes.

lexdevelop commented 9 years ago

Try to use MyISAM over InnoDB for session table.

CodeBrauer commented 9 years ago

@StefanLex , thanks for that tip. I set the table now in MyISAM - maybe that will do the trick.

ahmed-mahdi-pt commented 6 years ago

I'm using CI (2.1.3) which has this issue, and using database sessions. I've solved the issue. In the function sess_update(), instead of updating the session_id with new one, I insert the new session as new record, and the old one will expire and be deleted by garbage collection function _sess_gc(). So you need to do the following:

Comment out the below lines or remove them (lines 378-384):

    $cookie_data = array();
    foreach (array('session_id','ip_address','user_agent','last_activity') as $val)
    {
        $cookie_data[$val] = $this->userdata[$val];
    }

    $this->CI->db->query($this->CI->db->update_string($this->sess_table_name, array('last_activity' => $this->now, 'session_id' => $new_sessid), array('session_id' => $old_sessid)));

Replace them with the following:

    $custom_data = $this->userdata;
    $cookie_data = array();
    foreach (array('session_id','ip_address','user_agent','last_activity') as $val)
    {
            unset($custom_data[$val]);
            $cookie_data[$val] = $this->userdata[$val];
    }

    if (count($custom_data) === 0)
    {
            $custom_data = '';
    }
    else
    {
            $custom_data = $this->_serialize($custom_data);
    }

    $this->CI->db->query($this->CI->db->insert_string($this->sess_table_name, array('last_activity' => $this->now, 'session_id' => $new_sessid, 'ip_address' => $this->CI->input->ip_address(), 'user_agent' => substr($this->CI->input->user_agent(), 0, 120), 'user_data' => $custom_data)));

uchida-nunet commented 3 years ago

@ahmed-mahdi-pt

Thanks. It is useful information for me. You are my hero.

bcit-ci / CodeIgniter

Rapid requests during session updates trigger unexpected session destruction #154

Call reset() to ensure we start on a clean slate.

Call initialize() to seed a session value.

Wait 5 seconds so that the session will be forced to update on next request.

Call test() twice in rapid succession, so that both requests will be sent before either is fully processed. I used jQuery & Firebug to submit these requests using $.get().

View the responses. The first will report that the session is alive, but the second and all subsequent requests will report that it is dead.