Ansego Posted June 29, 2014 Share Posted June 29, 2014 Hi guys, Have some questions about "SESSIONS". I am setting: $_SESSION['sessionID'] = session_id(); BUT not sure how to use it correctly or what for? Is code below acceptable? Any suggestions for improvements? <?php error_reporting(-1); ini_set('display_errors', 'On'); spl_autoload_register(function ($class) { include 'lib/' . $class . '.inc'; }); session_start(); // =============================================================== if (isset($_GET['msg'])){ echo $_GET['msg']; } // =============================================================== if (isset($_POST['btn_login']) && !empty($_POST['btn_login'])){ $_SESSION['logged'] = "true"; $_SESSION['sessionID'] = session_id(); session_write_close(); header( 'Location: http://localhost/work/index.php?msg=Logged in!' ) ; } // =============================================================== if (isset($_POST['btn_logout']) && !empty($_POST['btn_logout'])){ //true $_SESSION['logged'] = ""; $_SESSION['sessionID'] = ""; session_write_close(); session_destroy(); header( 'Location: http://localhost/work/index.php?msg=Logged out!' ) ; } ?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <title>Server Status</title> </head> <body> <?php // =============================================================== if (!$_SESSION['logged'] == true){ ?> <form action="#" method="post" enctype="application/x-www-form-urlencoded" name="user-login"> <input name="btn_login" type="submit" value="Logon" /> </form> <?php }else{ ?> <form action="#" method="post" enctype="application/x-www-form-urlencoded" name="user-login"> <input name="btn_logout" type="submit" value="Logout" /> </form> <?php } ?> </body> </html> Kind regards and thanks. Quote Link to comment Share on other sites More sharing options...
Ch0cu3r Posted June 29, 2014 Share Posted June 29, 2014 I am setting: $_SESSION['sessionID'] = session_id(); BUT not sure how to use it correctly or what for? Why? There is no need to do that. This is why you have session_id() function for. The only time you need to get the session id is if you're doing something specific with it, such as writing your own session handler. Is code below acceptable? For what? Any suggestions for improvements? Yes, after calling header() make sure you also terminate script execution directly after. I hope there is more logic for processing the users login than what you currently show. The following will cause a undefined index notice if $_SESSION['logged'] is not defined if (!$_SESSION['logged'] == true){ You should check it exists first before checking its value if (isset($_SESSION['logged']) && $_SESSION['logged'] !== true){ The enctype="application/x-www-form-urlencoded" form attribute is only required if you are allowing file uploads when the form is submitted. I dont see you needing this for logging in/out users? I wouldn't pass messages over the url. If you are needing to do so then look into setting up flash messages (see here for an example) Quote Link to comment Share on other sites More sharing options...
Ansego Posted June 29, 2014 Author Share Posted June 29, 2014 Thanks, I assumed "session_id()" had something to do with the security of the session, thanks for straightening that up, was scratching my head on it for awhile. The enctype="application/x-www-form-urlencoded" form attribute is only required if you are allowing file uploads when the form is submitted. Thought encryption type "multipart/form-data" was for file uploads and "text/plain" was for debugging? All good, I'll start using "multipart/form-data" I hope there is more logic for processing the users login than what you currently show. Nope, that was it, was just working out sessions to have a better grasp. Is code below acceptable? Acceptable to use for a website session. Or has it got security breech all over it? Put the exit(); under the redirection headers. The GET was just for a quick msg, if someone wants to put junk in there it only echo's back ( Unless that is a breech? ). Thanks for your feed back mate, fixing the code up now. Quote Link to comment Share on other sites More sharing options...
Ansego Posted June 29, 2014 Author Share Posted June 29, 2014 fyi your IF statement failed: Failed: if (isset($_SESSION['logged']) && $_SESSION['logged'] !== true){ Operators !== failed. Works: if (isset($_SESSION['logged']) && !$_SESSION['logged'] == true){ Works: if (isset($_SESSION['logged']) && $_SESSION['logged'] != true){ Quote Link to comment Share on other sites More sharing options...
Ch0cu3r Posted June 29, 2014 Share Posted June 29, 2014 I assumed "session_id()" had something to do with the security of the session, thanks for straightening that up, was scratching my head on it for awhile. The session id is a unique id which is used to identify the user that belongs to the session. When you call session_start(); PHP will first check to make sure if a current session is active, by first looking to see if a PHPSESSID cookie is set (other places could be a PHPSESSID url query string parameter or form field named as such). If it cannot find the PHPSESSID. It will generate a new session, and create a new PHPSESSID cookie. What can happen with sessions is something called session hijacking, where by a malicious user manages to steal your PHPSESSID. All the malicious user would need to do is to create a PHPSESSID cookie, or define it as query string parameter/form field and the server will think it is you. Have a read of this chris shiflett article to understand it more and how to protect yourself. Thought encryption type "multipart/form-data" was for file uploads and "text/plain" was for debugging? All good, I'll start using "multipart/form-data" Miss read your code originally. application/x-www-form-urlencoded is the default enctype handled by the web browser. The only time you need to define the encytype as multipart/form-data is when you are going to be uploading files. So if you are not uploading files there is no added benefit for defining the enctype. The GET was just for a quick msg, if someone wants to put junk in there it only echo's back ( Unless that is a breech? ). .. and that will lead to Cross Site Scripting attacks . This does not apply to GET but all request inputs (such as POST and COOKIE etc..). No data retrieved from the user should ever be trusted. Quote Link to comment Share on other sites More sharing options...
Solution Jacques1 Posted June 29, 2014 Solution Share Posted June 29, 2014 The original code is wide open to session fixation attacks, because it happily adopts any user-provided ID. An attacker doesn't even have to steal the ID. They can make up their own (or get a fresh ID from you), trick the victim's browser into using it and wait for the victim to log-in. After this, they have a fully authenticated session. That's what needs to be fixed first. The problem is that the PHP session API doesn't distinguish between starting a new session and resuming an existing session. When you call session_start(), you get any of the following: a new session with a new ID a new session with an old ID provided by the user an old session When you authenticate a user, you certainly do not want to reuse an old ID or even run into an an existing session. You want a fresh session with a new ID. The current workaround for this is to reset the ID with session_regenerate_id() while making sure that no old content is carried over to the new session: <?php /* * Starting a new session. * * We can't just use session_start(), because it may do any of the following: * - start a new session with a new ID * - start a new session with a user-provided ID (which enables session fixation attacks) * - resume an existing session */ session_start(); // PHP may have resumed an existing session. Make sure we don't carry over any data to the new session. $_SESSION = array(); // Reset the session ID to make sure we don't reuse a known ID. The argument tells PHP to delete the old session. session_regenerate_id(true); // *Now* we have a new session. Write the data to $_SESSION as usual. Session hijacking is indeed a problem, but most advice on how to protect against it is incredibly naive, including the Shifflet article above. There are two ways how an attacker might obtain the session ID of another user: by listening to the network traffic by exploiting security holes on your site, in particular cross-site scripting vulnerabilities To protect against network attacks, use HTTPS at all times and set the Secure flag of the session cookie (this means it will only transmitted over HTTPS). Way too many sites still run around with plain HTTP, making it easy for an attacker to steal all kinds of data. In the second case, session hijacking is really your least problem. In fact, if an attacker has found a cross-site scripting vulnerability, they don't need the session ID. They can do (almost) anything they want with the account simply by controlling the victim's browser through JavaScript code. So it's naive to assume that the attacker is primarly interested in getting the ID so that they can then log-in as that user. Why should they? An automated attack from the victim's browser itself is much more efficient. The only case where they can't do that is if the session is valid for multiple domains and they want to read data from a different domain. This isn't possible with JavaScript due to the same-origin policy, so the attacker will indeed try to get the session ID. This can be prevented by setting the HTTPOnly flag of the session cookie (which means the cookie can't be read by JavaScript). This user agent stuff does not help. An attacker who has managed to obtain the session ID of a victim gets the user agent for free. And even if they don't know the UA, they'll simply try out all possiblities. There aren't too many. Wrapping it up: Fix your session initialization. Use HTTPS at all times and set the Secure flag of the session cookie. Set the HTTPOnly flag as well. Double-check that you don't have any cross-site scripting vulnerabilities in your code. In addition to that, you should remove all inline scripts and use Content Security Policy. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.