sessions safe for admin pages?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
wurdup
Forum Commoner
Posts: 39
Joined: Thu Apr 01, 2010 11:36 am

sessions safe for admin pages?

Post by wurdup »

Hi

I'm using

Code: Select all

session_start();
if (!isset($_SESSION['user']))
{
 die ("Access Denied");
}


and also

Code: Select all

session_start();
if ($_SESSION['USER'] != 'admin')
{
 die ("Access Denied");
}

for user pages and admin pages. Is this technique safe? Can the session by easily spoofed for the admin page? Session are set using a login page with psuedo code being:

if (password correct) {

session_start();
$_SESSION['user']= $username;

}

Thanks for any help.
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: sessions safe for admin pages?

Post by flying_circus »

In this case, asking if your code is safe, is akin to asking if peanut butter tastes good. You will get different responses from different people.

Where is your session data stored? If it is located in a shared directory, then it may be modified by a 3rd party.

Rather than checking if a session value is set, you should also validate it. Your second example validates the session variable, your first example doesnt.

You might give some thought to adding a few more mechanisms to complicate an attack. If I were to randomly guess the session id of a session with a 'user' as admin, then I would likely have admin control. It is impossible to be 100% sure the returning visitor is the same visitor who started the session. You can try tieing the session to the users user agent, as its not likely (but can) change between requests. I dont like to tie a session to a users IP, but some people are happy to do it. You can assign a random token to the user, so that they must send you a session cookie and also pass along a token on each request as well.

You're on the right track. Sessions are the way to go when tracking users, but I like to complicate the process as an effort to maintain "security in depth".
wurdup
Forum Commoner
Posts: 39
Joined: Thu Apr 01, 2010 11:36 am

Re: sessions safe for admin pages?

Post by wurdup »

yes I need absolute security for the sessions and would like to make things as difficult as possible. I will look into cookies and IPs for the admin as there is sensitive data there. I'm new to sessions in php so wasnt sure how secure they are.

Thank you
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: sessions safe for admin pages?

Post by Benjamin »

I don't feel that you are qualified to build a secure system. I also feel that the code base will have severe vulnerabilities when completed. Be advised that you could be opening yourself up to legal and civil penalties.
wurdup
Forum Commoner
Posts: 39
Joined: Thu Apr 01, 2010 11:36 am

Re: sessions safe for admin pages?

Post by wurdup »

Qualified or not it has to be done and I'm responsible enough to insure it is secure. Do you say that to every beginner on here as your comments aren't helpful.
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: sessions safe for admin pages?

Post by kaisellgren »

Well, you've got the very basics. The values in the session can't be spoofed easily by an arbitrary cracker. If this is on shared hosting, the session data files may be subject to modification by other clients on the server. If not, then it's quite unlikely to have anyone altering your session data.

Unfortunately security is a huge topic. You should consider using SSL/TLS. You should protect from session fixation and mitigate session hijacking attacks. Protect the session storage well, and even if you do all this the system may be cracked another way (for example, getting the session identifier via XSS, or using SQL injection to crack into the admin system) among many other things.

One hole somewhere in your application could give an access to your admin system. It's not just about the back-end. It's about the entire software, server, users, configuration, transport.

We have no idea what are you doing this for. If you are creating a site with a high value, then Benjamin is right about you not being qualified. On the other hand, if this is your own personal home page or just some simple project like a school project, it may not even hurt yourself if the code is insecure.
wurdup wrote:Do you say that to every beginner on here as your comments aren't helpful.
Maybe they were helpful, maybe they weren't, but at least we have now considered that aspect and you know not to code for a bank so let's not start a war. ;)
wurdup
Forum Commoner
Posts: 39
Joined: Thu Apr 01, 2010 11:36 am

Re: sessions safe for admin pages?

Post by wurdup »

No I'm not coding for a bank simply building a secure system for myself that may potentially hold data once complete.There will be lots of testing and research before that happens. I've decided to do it myself after a so called 'qualified' person left a password recovery exploit on a page and the website got hacked and used for phising.
xtiano77
Forum Commoner
Posts: 72
Joined: Tue Sep 22, 2009 10:53 am
Location: Texas

Re: sessions safe for admin pages?

Post by xtiano77 »

Wurdup,

I am building a site for the same reason as you. That said, I use a combination of $_SESSION variables and a database tables to secure my site. I don’t know if this is the best way to go about it, but it seems to work for me. Perhaps if you are like me in that sometimes seeing how things are implemented help me to understand better what is being said. One caveat though, I use OOP so please be patient with my coding style. Also if you find a better solution to this riddle, please reply and share it with the group. Cheers!

The first section of code is a Class named “Sessions” (you can name it whatever you want, I chose the name based on the topic of this tread). It declares three properties and several methods which are all used to establish and later to verify the validity of the same. Basically, once the session has been established, normally I do this upon successful authentication of the user, every time the “$obj -> validateSession( )”, it checks that the values of:

$_SESSION[“USERNAME”],
$_SESSION[“UNIQUESESSIONID”], and
$_SERVER[“HTTP_USER_AGENT”]

match what is stored in the database. If the values do not match then the page will be redirected to the “loggout.php” page where the “$obj -> destroySession( )” will be called preventing the user from returning to the site by clicking on the “Back” button.


Sessions.php

Code: Select all

<?php
require_once("Database.php");
class Sessions {
	private $_db;
	private $_sessionId;
	private $_browser;

	public function __construct(){
		try{
			$this -> _sessionId	= sha1(uniqid() . pi()); // this is just an example of how you could create a unique session id
			$this -> _browser	= $_SERVER["HTTP_USER_AGENT"];
			$this -> _db		= new Database();
			if(!is_resource($this -> _db)){
				throw new Exception("UnableToCreateSessionValues");
                                                }
		}catch(Exception $e){
			header("Location: http://www.yoursite.com/loggout.php?exception=true&message=" . $e -> getMessage());
		}
	}

	private function updateSessionTable($username = ""){
		$query = "INSERT INTO yourdatabase.yourtable (id, username, sessionId, browserInformation) VALUES (NULL, '" . $username . "', '" . $this -> _sessionId . "', '" . $this -> browser . "')";
		$result = $this -> _db -> queryDatabase($query);
		if(!$result != 1){
			throw new Exception("UnableToUpdateSessionTables");
		}			
	}

	private function setSessionTimer(){
		$_SESSION["TIMER"] = time() + 1200;
	}

	private function checkSessionValidity(){
		$query = "SELECT * FROM yourdatabase.yourtable WHERE username = '" . $_SESSION["USERNAME"] . "' AND sessionId = '" . $_SESSION["UNIQUESESSIONID"] . "' AND browserInformation = '" . $this -> _browser . "'";
		$result = $this -> _db -> queryDatabase($query);
		if(!is_resource($result)){
			throw new Exception("InvalidSession");
		}
	}

	private function checkSessionTimer(){
		if($_SESSION["TIMER"] < time()){
			throw new Exception("SessionExpired");
		}
		$this -> setSessionTimer();
	}

	public function createSession(Database $resource){
		try{
			// session variable 1
			// session variable 2
			// session variable 3
			// ...
			// cookie(...);
			// cookie(...);

			$this -> setSessionTimer();
			$this -> updateSessionTable("theUserNameOfThePersonUsingTheSite");
		}catch(Exception $e){
			header("Location: http://www.yoursite.com/loggout.php?exception=true&message=" . $e -> getMessage());
		}
	}

	public function destroySession(){
		$_POST		= array();
		$_GET		= array();
		$_SESSION	= array();
		session_destroy();
		cookie("PHPSESSID", "", time90 - 1200, "/", ".yoursite.com"); 
	}

	public function validateSession(){
		try{
			$this -> checkSessionTimer();
			$this -> checkSessionValidity();
		}catch(Exception $e){
			header("Location: http://www.yoursite.com/loggout.php?exception=true&message=" . $e -> getMessage());
		}
	}
}
?>
yourPage.php

Code: Select all

<?php
session_start();

$api = "/your/directory/here/";
require_once($api . "Sessions.php");

$session = new Sessions();
$session -> validateSession();

// the rest of your code goes here...
?>
Last edited by xtiano77 on Mon May 10, 2010 3:12 pm, edited 1 time in total.
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: sessions safe for admin pages?

Post by flying_circus »

What are you trying to accomplish with your session class? Much of it seems redundant and I foresee it throwing E_NOTICE
xtiano77
Forum Commoner
Posts: 72
Joined: Tue Sep 22, 2009 10:53 am
Location: Texas

Re: sessions safe for admin pages?

Post by xtiano77 »

I don't have my Server available right now so this was on the fly in an effort to answer the original question. I though it was explained above, but obviously I didn't communicate that in a meaningful way. The class declares properties and methods which are used to establish a unique session id for the current user. That session id is stored in a table along with the username and the browser information. Later, when a page is accessed a method will try to match the $_SESSION["UNIQUESESSIONID"] and the $_SERVER["HTTP_USER_AGENT"] to what is stored in the database, if no match is found then the user will be redirected to the loggout page where the session will be destroyed by another method. I hope this clarifies some. If you don't mind, could you point out the line[s] of code you suspect will give/throw an E_NOTICE?
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: sessions safe for admin pages?

Post by flying_circus »

I haven't looked at your code too closely, but you've got a syntax error on line 13. There looks to be a another syntax error on line 68. After the object is created, you call validateSession() which calls checkSessionTimer(). If this is a new session, $_SESSION["TIMER"] wont exist and will throw an E_NOTICE or an exception that the session has expired. I assume you'd have to call createSession() at some point.
xtiano77 wrote:I don't have my Server available right now so this was on the fly in an effort to answer the original question. I though it was explained above, but obviously I didn't communicate that in a meaningful way. The class declares properties and methods which are used to establish a unique session id for the current user. That session id is stored in a table along with the username and the browser information. Later, when a page is accessed a method will try to match the $_SESSION["UNIQUESESSIONID"] and the $_SERVER["HTTP_USER_AGENT"] to what is stored in the database, if no match is found then the user will be redirected to the loggout page where the session will be destroyed by another method. I hope this clarifies some. If you don't mind, could you point out the line[s] of code you suspect will give/throw an E_NOTICE?
Why create a "unique session id" when a session id is generated when the session is started? The native session implementation has a pretty strong random for the id. I understand the concept of trying to tie a session id to a user agent.

Have you looked into creating a custom session handler? I think you'd probably end up with a cleaner solution.
xtiano77
Forum Commoner
Posts: 72
Joined: Tue Sep 22, 2009 10:53 am
Location: Texas

Re: sessions safe for admin pages?

Post by xtiano77 »

You were correct about line 13, I missed a "}". However, I don't understand what you mean about the "checkSessionTimer()" and the "validateSession()". The "checkSesstionTimer()" is called by the "validateSession()" on subsequent pages and only after the session data has been called by "session_start()". As far as the method mechanics, it basically checks the timer inside the variable and if it is less than the current time, it redirects the user to the logout page, otherwise it calls "setSessionTimer()" and adds another 20 minutes to the present time. Can you be so kind as to provide a link or a code example of a better session handling function? I can definetely use a better way of doing things.
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: sessions safe for admin pages?

Post by flying_circus »

I guess the point of what I was trying to say was to always check for existence before referencing something:

Code: Select all

        private function checkSessionTimer(){
                //if($_SESSION["TIMER"] < time()){
                if(isset($_SESSION["TIMER"]) && $_SESSION["TIMER"] < time()) {
                        throw new Exception("SessionExpired");
                }
                $this -> setSessionTimer();
        } 
Chris Shiflett has written a nice article which may be helpful as well: http://shiflett.org/articles/storing-se ... a-database

Code: Select all

<?php
  # Session Related Constants
    define("SESSION_MAX_LIFETIME", ini_get("session.gc_maxlifetime")); // 48 Hours
    define("SESSION_ID_MAX_LIFETIME", 1440); // 24 minutes
  
  class Session
  {
  # Variables
    private $session = array();
    
    function __construct()
    {
    # Requires
      if(!Registry::isRegistered("core"))
        exit("Class::Session requires: Class::Core.  Class::Core not found.");
        
      if(!Registry::isRegistered("database"))
        exit("Class::Session requires: Class::Database.  Class::Database not found.");
        
    # Build Fingerprint
      $this->session['fingerprint'] = md5(Registry::__get('core')->__get('client_user_agent'));
      
    # Begin Session
      session_set_save_handler(array($this, "open"),
                               array($this, "close"),
                               array($this, "read"),
                               array($this, "write"),
                               array($this, "destroy"),
                               array($this, "gc"));
                               
      session_start();
      
    # Should Session Id be rotated?
      if(isset($this->session['created']) && $this->session['created'] < (time() - SESSION_ID_MAX_LIFETIME))
        session_regenerate_id(1);
        
    # Have we traversed protocols?
      if(!Registry::__get("core")->is_https() && $this->session['secure'] == 1) {
      # HTTPS Session -> HTTP Environment
        session_regenerate_id(1);
        $this->session['secure'] = 0;
      }
        
      if(Registry::__get("core")->is_https() && $this->session['secure'] == 0) {
      # HTTP Session -> HTTPS Environment
        session_regenerate_id(1);
        $this->session['secure'] = 1;
      }
    }
    
    function open()
    {
      return true;
    }
    
    function close()
    {
      return true;
    }
    
    function read($id)
    {
      $results = Registry::__get("database")->query(sprintf("SELECT `session_created`, `session_secure`, `session_data` FROM `sessions` WHERE `session_id`='%s' AND `session_fingerprint`='%s' LIMIT 1;",
                                                            Registry::__get("database")->escape($id),
                                                            Registry::__get("database")->escape($this->session['fingerprint'])));
                                                            
      if($results && $results->num_rows == 1) {
        $session = $results->fetch_assoc();
        $this->session['created'] = $session['session_created'];
        $this->session['secure'] = $session['session_secure'];
        return $session['session_data'];
      } else {
        $this->session['created'] = time();
        $this->session['secure'] = (Registry::__get("core")->is_https()) ? 1 : 0;
      }
      
      return '';
    }
    
    function write($id, $data)
    {
      return Registry::__get("database")->query(sprintf("INSERT INTO `sessions` (`session_id`, `session_created`, `session_fingerprint`, `session_secure`, `session_data`) VALUES ('%s', '%u', '%s', '%u', '%s') ON DUPLICATE KEY UPDATE `session_data`='%s';",
                                                        Registry::__get("database")->escape($id),
                                                        Registry::__get("database")->escape($this->session['created']),
                                                        Registry::__get("database")->escape($this->session['fingerprint']),
                                                        Registry::__get("database")->escape($this->session['secure']),
                                                        Registry::__get("database")->escape($data),
                                                        Registry::__get("database")->escape($data)))
                                                        ? mb_strlen($data) : 0;
    }
    
    function destroy($id)
    {
      return Registry::__get("database")->query(sprintf("DELETE FROM `sessions` WHERE `session_id`='%s' LIMIT 1;",
                                                        Registry::__get("database")->escape($id)));
    }
    
    function gc($maxlifetime)
    {
      return Registry::__get("database")->query(sprintf("DELETE FROM `sessions` WHERE `session_created` < '%u';",
                                                        Registry::__get("database")->escape(time() - $maxlifetime)));
    }
  }
?>
xtiano77
Forum Commoner
Posts: 72
Joined: Tue Sep 22, 2009 10:53 am
Location: Texas

Re: sessions safe for admin pages?

Post by xtiano77 »

I sure appreciate your comments and input. I will definetely review your last post and read Shifflet's article. Thanks again!
Post Reply