CodeIgniter Forums
NGSession security issues - Printable Version

+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Archived Discussions (https://forum.codeigniter.com/forumdisplay.php?fid=20)
+--- Forum: Archived Libraries & Helpers (https://forum.codeigniter.com/forumdisplay.php?fid=22)
+--- Thread: NGSession security issues (/showthread.php?tid=6924)



NGSession security issues - El Forum - 03-17-2008

[eluser]kirilisa[/eluser]
Hi folks,

I was just looking at the code in NGSession to see how it works and I came across a couple things that puzzled me/seemed not right.

1) In the function sess_update, it generates a new session id and a new last activity time, and then calls sess_write_database or sess_write_cookie with that information. HOWEVER, in sess_write_database it explicitly does *not* write session id information (it unsets any session_id info passed to it): ergo, the new session id is never saved to the DB, although the new last activity time is, thus (I would think) completely defeating the purpose of session update. And indeed, when I've tested it, I see the new session id generated, but when I check my database, that information is never written there. This seems like a pretty bad bug to me.


2) In function sess_read, when it is looking up the session data from the database (based on the session id it got from the cookie) the SQL statement has the following WHERE clauses:

Code:
// database mode - no need to check if all fields are required - just get them all                                                                                      
    if ($this->use_database == TRUE)
      {
        $this->CI->db->where('session_id', $this->session_id);
        $this->CI->db->where('ip_address', $this->CI->input->ip_address());
        $this->CI->db->where('user_agent', trim(substr($this->CI->input->user_agent(), 0, 50)));

Now this is *before* it checks to see if you have indeed set sess_match_ip and sess_match_useragent to be TRUE, i.e. it seems to me that even if you didn't want Useragent and IP matching to happen, they are still happening? What's odd is that farther down in function sess_read it does do a check on sess_match_ip and sess_match_useragent - using the same values as it used in the WHERE clause.

Code:
// Does the IP Match?                                                                                                                                                    
    if ($this->CI->config->item('sess_match_ip') == TRUE AND $session['ip_address'] != $this->CI->input->ip_address())
      {
        $this->sess_destroy();
        return FALSE;
      }

    // Does the User Agent Match?                                                                                                                                            
    if ($this->CI->config->item('sess_match_useragent') == TRUE AND trim($session['user_agent']) != trim(substr($this->CI->input->user_agent(), 0, 50)))
      {
        $this->sess_destroy();
        return FALSE;
      }

This is a redundancy, is it not? These if statements will never return TRUE because the WHERE clause makes it impossible that they'd be different, no? Am I missing something? I can't really test it, but it seems to me that given the WHERE clause, retrieving the session data will always fail if your Useragent or IP have changed, whether you set the config to be rigorous in that way or not.

Please correct me if I am wrong!

Thanks!


NGSession security issues - El Forum - 03-17-2008

[eluser]kirilisa[/eluser]
Whoops sorry guys was trying to post this in the NGSession thread but hit New Topic by mistake! I dunno how to move it :-o


NGSession security issues - El Forum - 03-18-2008

[eluser]esra[/eluser]
Write a post in the NGSession thread with a link to this post.


NGSession security issues - El Forum - 03-26-2008

[eluser]WolfgangA[/eluser]
Continued here.