9

I know this has been asked billions of times, but I'm super paranoid/OCD about the security of my coding. I'm working on a little project. The session data will only contain:

user_id 1
user_name MyUsername
logged_in true
csrf_token 87cc51ee94178df79cccce2aebc45d53

Here's my code. Is it secure enough to use on a small CMS?

session_start();

ini_set('session.cookie_httponly', 'On');
ini_set('session.cookie_secure', 'On');
ini_set('session.use_cookies', 'On');
ini_set('session.use_only_cookies', 'On');

$rand = rand(1, 10);

if ($rand != 1 || $rand != 3 || $rand != 5)
    session_regenerate_id();

$user_ip = md5($_SERVER['REMOTE_ADDR']);
$user_agent = md5($_SERVER['HTTP_USER_AGENT']);

if (isset($_SESSION['user_ip'], $_SESSION['user_agent'])) {
    $session_user_ip = $_SESSION['user_ip'];
    $session_user_agent = $_SESSION['user_agent'];

    if ($session_user_ip != $user_ip || $session_user_agent != $user_agent) {
        unset($_SESSION);
        session_destroy();

        die('Error');
    }
} else {
    $_SESSION['user_ip'] = $user_ip;
    $_SESSION['user_agent'] = $user_agent;
}

Then to call the sessions:

$_SESSION['user_id'] = 1;
$_SESSION['user_name'] = 'MyUsername'; // etc.

Extra Info
I'll be using the session data to check if user has permissions to do something. Example: if ( user_has_perm( $_SESSION['user_id'] ) )

Thanks for your help in advance.

ck_
  • 3,353
  • 5
  • 31
  • 33
user1453094
  • 373
  • 2
  • 7
  • 13
  • 1
    And your question exactly is..? – samayo Jan 14 '13 at 14:44
  • 2
    @PHPNooB Quote: `Here's my code. Is it secure enough to use on a small CMS?` – user1453094 Jan 14 '13 at 14:45
  • 3
    I believe his question is "Is it secure enough to use on a small CMS?", although it requires a little more detail. That piece, by itself, is secure. If by secure you mean that the session will be reset if the ip or user agent changes... – aurbano Jan 14 '13 at 14:46
  • Thanks @Chevi. So this is safe to use on a live site? – user1453094 Jan 14 '13 at 14:47
  • 'safe enough'? That's not a valid question. We're supposed to know what your threat parameters are? Is it safe enough to keep you from breaking in? Yes. schneier's law demands that. Is it safe to keep a script kiddy out? Maybe. Safe enough to keep a large government agency out? Hell no. – Marc B Jan 14 '13 at 14:50
  • Nothing is safe in the digital world. – samayo Jan 14 '13 at 14:52
  • I would say that this snippet is good to go live security wise (I would probably implement the session regeneration little differently, but that's not much of an issue). What we cannot say is how it is actually implemented in the CMS, how you check CSRF tokens etc., thus if your CMS is actually secure, that is a whole other question... Also the question could probably be better rephrased as 'is my session management in a web application correctly implemented?' which would've been a little clearer. – cyber-guard Jan 14 '13 at 14:52
  • I think its the other way around: is it safe enough to keep a large government agency out? Always. :P – mishmash Jan 14 '13 at 14:53
  • 1
    Also you should not directly `unset($_SESSION);` – Lawrence Cherone Jan 14 '13 at 14:54
  • User Agent and IP can both be spoofed in the HTTP requests. I'm not sure that checking anything to do with them is even worth the processing power. – crush Jan 14 '13 at 14:55
  • Haha, thanks for all your answers, very helpful! Unfortunately, they're only comments so I can't mark a best answer. :( Will fix that @LawrenceCherone – user1453094 Jan 14 '13 at 14:55
  • Work with abstraction, especially through object oriented programming. – Leonard Jan 14 '13 at 15:00
  • 1
    @Zanathel I'm not sure how that solves the question of security here. – crush Jan 14 '13 at 15:03
  • @crush: well in terms of session hijacking it makes it a little more difficult (although the useragent bit is really easy), but it is extremely difficult, if possible at all, to spoof `$_SERVER['REMOTE_ADDR']` to an exact address, so actually it is useful. – cyber-guard Jan 14 '13 at 19:51

7 Answers7

13

Session security risks come from three different possibilities:

  • Prediction
  • Capture
  • Fixation

Prediction would mean that someone that's not the user for whom the session was created guessed their session ID. The chances of that happening are almost 0, although they do grow as more users use the site simultaneously.

With your code, you would make that risk even lower because it would only work if the attacker shared the user agent and the ip of the predicted session. But the difference is trivial in this case.

Fixation would mean that an attacker can create a session and then force another user into using their session. In this case it would depend: If the attacker knows that you are doing it and they fake the user agent and ip of the client, they could fixate the session. Or if they share ip and user agent.

And finally we have session hijacking, probably the most common method of the three. In this case an attacker would somehow gain access to the session id of a valid logged in user, and then use it to log in to their account. As with the previous method, this would only work for them if they know that you are checking the ip and user agent, and faked the same ones as the user. The technique you are using is not unique, and some attackers might fake them just in case.

That being said, is it secure? Yes and no

If you are obsessed with security, the answer is always the same: Use SSL

Unless your code is open source, almost anything you do that changes the behavior of the php sessions will be secure enough.

The only exception to that would be really popular sites that will attract the attention of hackers.

There is some very good documentation on this topic available:

Community
  • 1
  • 1
aurbano
  • 3,324
  • 1
  • 25
  • 39
  • A common approach also is to store the session id (in a database for instance), and invalidate it upon logon of another authenticated session from the same user - so that the hijacker only has access so long as the original creator of the session does not create a new session. – crush Jan 14 '13 at 15:01
  • Good answer +1! However, the main, giant risk is the theft of the credentials, which makes this discussion about sessions completely useless, unless you use SSL only for authentication, and then go on by session auth. – gd1 Jan 14 '13 at 15:02
  • 1
    Completely true @gd1, worrying about session security at his stage is from my point of view trivial. The real security risk is not that. – aurbano Jan 14 '13 at 15:05
  • @crush I'm not sure if that option is good, sometimes I might have my desktop computer and laptop logged in to the same computer. Enabling only one login at a time might only be viable for some specific applications – aurbano Jan 14 '13 at 15:06
  • Storing irreversible, hashed passwords with a randomly generated salt of 30+ characters, and encrypted with a private key that is hosted on a remote server seems to be the commonly accepted way of keeping users data safe in the event of a server-side data breach. – crush Jan 14 '13 at 15:06
  • @crush I was referring to your first comment on my answer, when you suggested that a common approach would be to enable only one login at a time. – aurbano Jan 14 '13 at 15:08
  • Thanks for the answer @Chevi. And all your comments to everyone above. I didn't expect to get this much out of posting a question but I've learnt a great deal. Thanks so much everyone! – user1453094 Jan 14 '13 at 15:09
  • @Chevi My last comment wasn't in response to your comment. Your comment is correct - and is given as the case when such an approach couldn't be enforced. – crush Jan 14 '13 at 15:10
  • The original question is very badly phrased - security means different things to different people. This answer is not complete - for example it fails to address the possibility of direct attacks on the session data (which can occur on a shared server) – symcbean Jan 14 '13 at 15:19
  • Ad prediction: actually not so long ago facebook was subject to this attack, because of low amount of entropy used in creating in the PHP sessions @symcbean: I wouldn't say that the answer fails to address the possibility because the question was asked in the context of code implementation. No code is ever fully secure if server is insecure, or on poorly configured shared server... – cyber-guard Jan 14 '13 at 19:59
  • But there are lots of ways of making the session data non-searchable. – symcbean Jan 14 '13 at 23:56
4

I'm not a security expert. However, I humbly doubt that your security enforcements will bring substantial benefits.

If there's one who can steal the session ID of your users, for example by eavesdropping an unencrypted wireless network, I bet he can steal also the username and password your users send to your server when they authenticate. Once he has the access credentials, the attacker can login the day after, or a week after, and will have his "safe" - and 100% valid - session to play with.

I believe there is no session security without channel security. If you use SSL, you ensure that the session ID is sent only via cookies (you're already doing it) and your sessions expire soon, I believe you are reasonably safe, and safer than making these enforcement on an insecure channel.

gd1
  • 11,300
  • 7
  • 49
  • 88
  • 1
    Thanks for the answer! Unfortunately, I had to mark Chevi's as best though, it was much more informative. I'm grateful for yours though! – user1453094 Jan 14 '13 at 15:12
3

Firstly, you have a mistake in the session regenerate code. The following if will always evaluate to true:

if ($rand != 1 || $rand != 3 || $rand != 5)

If $rand is not 1, it returns true. If $rand is 1, then it's not three, and it returns true. You probably meant to use an and here.

Secondly, you don't need to MD5 the user_ip, or the user_agent strings. If someone can access the session data on your server directly, you're so deep in it that hashing that data won't save you.

CLARIFICATION: As SDC and crush point out in the comments, MD5 is good for hashing passwords if you hash it with a salt. This means that your user's passwords are generally still secure, even if a SQL Injection attack succeeds and your database is exposed to the world. However, if your server is compromised, and the salt is compromised, then it becomes possible to generate a set of known hashes, and to attempt a reverse lookup of a specific password. Bottom line? Hash your user passwords, with a salt.

Thirdly, most security holes don't come from spoofing sessions. They come from poor input checking. A book like Essential PHP Security should be a good introduction to the kind of input checking you should do in a PHP project. Failing that, at least read the security section of the PHP Manual. Pay attention to the SQL Injection bit. It's cool!

Finally, I fully agree with the other poster that you should use SSL to secure communication to your website.

Gustav Bertram
  • 14,591
  • 3
  • 40
  • 65
  • Oops! Thanks for pointing that out! Will check out those articles/books too, thanks again! :) – user1453094 Jan 14 '13 at 15:10
  • The answer states that md5 is good for hashing passwords. This is WRONG. md5 is **NOT** good for hashing passwords. It can be cracked in seconds by an even half-competent hacker. If you're using md5 for anything you consider secure, then you're making a massive mistake. – SDC Jan 14 '13 at 15:18
  • 2
    It is good for hashing passwords if you hash it with a salt. If that salt is compromised, though, then the hash becomes worthless. You are generally spreading ignorant information here. Yes, an MD5 hash by itself is insecure. There are ways to secure it. – crush Jan 14 '13 at 15:23
1

They are just only two major session attacks

1.) Session fixation Attacks.

This can be prevented by using session_regenerate_id()

2.) Session Hijacking:

This can be prevented via data encryption by using SSL Certificates. your site will now run on https and not http.

3.) If you are on a shared server where CloudFare or JailRoot is not implemented. You can show to store your sessions in a database as opposed to default file system storage.

With this three implementations, Let me see that hacker who claims to hack usesrs session...

Nancy Moore
  • 2,322
  • 2
  • 21
  • 38
0

To be honest, I think you're being over-cautious, but not in a very useful way.

If you're really worried about session security, don't try to do it yourself. Use a PHP security patch like Suhosin. Let it do all the hard work for you. Suhosin is a well-established patch for PHP. It includes stuff that deals with all the ways a PHP session can be hacked. If you've got it installed, then you don't need to be doing anything extra to secure your session. If you don't have it installed, then you definitely can't claim to be super paranoid about security!

So in short, install Suhosin (if you haven't already), and forget about it.

But just for completeness, I'll make a few comments on at your actual code, just to point out a couple of things:

I'm not sure why you think MD5 hashing makes any difference. MD5 hashes can be cracked in seconds, so having them in any kind of security function is completely arbitrary. It may as well be plain text. That said, I don't really see the need for them to be anything other than plain text anyway -- if a hacker has managed to get hold of the session data to be able to read the IP address it contains, then you're already beyond worrying about whether they know the IP address or not.

SDC
  • 14,192
  • 2
  • 35
  • 48
  • MD5 hashes can be cracked in seconds? That's not necessarily true at all. It's true they can be reversed in "seconds" with the use of a Rainbow table, but that process wouldn't help an attacker in any way in this case. Not to mention, reversing a properly salted MD5 hash doesn't usually produce anything an attacker could use. The point about IP addresses and User Agents being pointless to check/store is true, though, because any hacker sophisticated enough to hijack the session could also spoof the user agent and ip. – crush Jan 14 '13 at 15:19
  • @crush - yes it it. http://en.wikipedia.org/wiki/MD5 -- quote: *The security of the MD5 hash function is severely compromised. A collision attack exists that can find collisions within seconds...* – SDC Jan 14 '13 at 15:21
  • Again, that's not true in every scenario. What are you going to collision attack against the concatenation of the user agent and ip? How will you test the success of the collision in a manner of seconds? LOL – crush Jan 14 '13 at 15:22
  • Both IP address and User agent have a (relatively) limited set of possible values, so I would expect it to be fairly trivial -- if you've got the session data already, then you know what the array keys are ("user_ip" and "user_agent") so you don't need to guess the content type. You don't even need to check for collisions for the IP address; you can just run through the 4 billion possible IPs (it won't take long, especially if you target the most likely ranges first). And as for the user agent, you can spoof your UA string to match a hash collision, and job done. MD5 is dead; accept it. – SDC Jan 14 '13 at 15:27
  • No. You don't have the session data. You have the session id. Then you have to find the right combination of IP address and user-agent to match up with that session id. There are thousands of collisions, sure. But you'd also have to submit an HTTP request for each possibility in order to find out if it was a match or not. Do you understand the concept of salts? Salts are a direct counter-measure for collision attacks against MD5. They nullify it completely with a decent salt. – crush Jan 14 '13 at 15:28
  • Well, I don't see a salt there (nor a concatenation for that matter). Also, the MD5 in this case only serves any purpose at all as a protection against someone who has obtained the session data -- if the attacker doesn't have the session data in this case, then the hash is completely irrelevant because neither the original values nor the hash are ever exposed. – SDC Jan 14 '13 at 15:32
  • I can agree with that. Relying on anything relayed from the client as being genuine and untainted is always a mistake. – crush Jan 14 '13 at 16:14
-1

Select image to upload: "; $res=mysql_query("SELECT `id`, `FirstName`, `LastName`, `Address`, `Password`, `Repassword`, `Birthday`, `Gender`, `user_image` FROM `registration`") or die(mysql_error()); while($row=mysql_fetch_array($res)) { $id=$row['id']; $firstname=$row['FirstName']; $user_image=$row['user_image']; $page.=" $firstname"; } $page.=" "; $res_post=mysql_query("SELECT post_info.post_info_id, post_info.id, post_info.post_info_desc, registration.FirstName FROM `post_info` join `registration` WHERE post_info.id = registration.id order by post_info.post_info_id desc") or die(mysql_error()); while($row_post=mysql_fetch_array($res_post)) { $post_id = $row_post ['post_info_id']; $post_desc = $row_post ['post_info_desc']; $id = $row_post ['id']; $FirstName = $row_post ['FirstName']; $page.="

$FirstName

$post_desc

"; } $page.=" "; include('include/main_file.php'); ?>
jb rocks
  • 1
  • 1
    Goodness gracious! I would really suggest some code formatting. Consecutive lines, nice indents - makes code SO much easier to read that way. Additionally, I am not sure what this actually has to do with the question asked? – Fluffeh Feb 05 '15 at 06:15
-2
<?php

session_start();
$con=mysql_connect("localhost","root","");
$seldb=mysql_select_db('myfreind', $con);
$email=$_POST['txtEmail'];
$password=$_POST['txtPass'];
$res=mysql_query("SELECT `id`, `FirstName`, `LastName`, `Address`, `Password`, `Repassword`, `Birthday`, `Gender` FROM `registration` WHERE 
`Password`='$password' and  `FirstName`='$email'");
$num=mysql_num_rows($res);

if($num==1)
{
$row=mysql_fetch_array($res);
$id=$row['id'];
$firstname=$row['FirstName'];
$_SESSION['id']=$id;
$_SESSION['FirstName']=$firstname;
//echo $_SESSION['id'];
header('Location:main.php');
}else

{

header('Location:index.php?error');
}

?>
Ankur Bhadania
  • 4,123
  • 1
  • 23
  • 38
jb rocks
  • 1