Hi folks,
I recently made a post about some security problems that I felt existed in NGSession - but I put the post as a new topic (I thought I’d hit Reply but whoops) by mistake and I don’t think I can move it.
Anyway, the topic is here if anyone wants to read it Thread 74428. In short, things that seem to me problems are:
1) When sess_update() is called, the new session ID does not actually get written to the DB, i.e. the session is never in fact updated [because userdata[‘session_id’] is unset & never stored in sess_write_database()]
2) NGSession does an IP matching check and User Agent matching check when selecting from the DB in sess_read() whether you have set those options to TRUE in your config file or not
3) I also felt that the names ip_address, user_agent, and last_activity were too generic and thus have a fairly decent chance of being overwritten manually by mistake so I renamed them (not shown in code below)
I updated the code to fix the above issues for my own purposes, but I thought I’d post the relevant methods here too, in case anyone wants them. The changes are really quite minor and are marked with //@@@.
Of course, if I have updated it wrong, or understood something wrong, please correct me!
This code only includes the relevant methods!
/* mysql table creation
*
* CREATE TABLE `ci_sessions` (
* `session_id` varchar(40) NOT NULL default '0',
* `sess_ip_address` varchar(16) NOT NULL default '0',
* `sess_user_agent` varchar(50) NOT NULL,
* `sess_last_update` int(10) unsigned NOT NULL default '0',
* `session_data` text,
* PRIMARY KEY (`session_id`)
* ) ENGINE=MyISAM DEFAULT CHARSET=utf8;
*/
function sess_read() {
...
// database mode - no need to check if all fields are required - just get them all
if ($this->use_database == TRUE) {
//@@@ check IP and user agent only if specified by config!
$this->CI->db->where('session_id', $this->session_id);
if ($this->CI->config->item('sess_match_ip') == TRUE) $this->CI->db->where('sess_ip_address', $this->CI->input->ip_address());
if ($this->CI->config->item('sess_match_useragent') == TRUE) $this->CI->db->where('sess_user_agent', trim(substr($this->CI->input->user_agent(), 0, 50)));
...
}
function sess_write_database($update=FALSE)
{
// get current user agent and IP address; format data to match database structure
$db_data = array
(
'sess_ip_address'=> $this->userdata['sess_ip_address'],
'sess_user_agent'=> $this->userdata['sess_user_agent'],
'sess_last_update'=> $this->userdata['sess_last_update']
);
//@@@ if this is an update, include new session id
if ($update) $db_data['session_id'] = $update;
// now serialize only the userdata part
$db_userdata = $this->userdata;
unset($db_userdata['session_id']);
unset($db_userdata['sess_ip_address']);
unset($db_userdata['sess_user_agent']);
unset($db_userdata['sess_last_update']);
$db_data['session_data'] = serialize($db_userdata);
// update database (matching against old session id)
$this->CI->db->query($this->CI->db->update_string($this->session_table, $db_data, array('session_id' => $this->session_id)));
//@@@ now set new session id for cookie writing
if ($update) $this->session_id = $update;
}
function sess_update()
{
$new_sessid = '';
while (strlen($new_sessid) < 32) {
$new_sessid .= mt_rand(0, mt_getrandmax());
}
$new_sessid = md5(uniqid($new_sessid, TRUE));
// Update the session data in the session data array
$this->userdata['session_id'] = $new_sessid;
$this->userdata['sess_last_update'] = $this->now;
//@@@ database mode - update the session id in the db
if ($this->use_database == TRUE) {
$this->sess_write_database($new_sessid);
}
// Write the cookie
$this->sess_write_cookie();
}I think that’s it. Hopefully I didn’t forget anything while cutting and pasting!