0

I am experimenting with sessions.

I am trying to get it to be a secure session, I have the user only receiving a session variable of "signin"equal to 5 only after they have been verified through salting and hashing.

 if($passwordhash == $tablehash)
    {
            session_start();
            $_SESSION['signin'] = 5;
            header("Location: /~cssgf3/cs3380/lab7/home.php");

And then I have the page that it gets redirected to being verified through this step:

session_start();

$sessionValue = 5;//$_SESSION['sessionValue'];
$userSession = $_SESSION['signin'];

if($userSession == $sessionValue)
{

I am curious if this is an appropriate way of authenticating a session. I'm not too worried about security at this point, but I am interested if this is at all secure and if not if theres a somewhat easy way to fix it?

Flexo
  • 87,323
  • 22
  • 191
  • 272
ZAX
  • 968
  • 3
  • 21
  • 49
  • the verification through salting and hashing is where any sort of security would occur, but really you should `set the user session to a user specific value that will change on each login` and `verify these session/DB values on every page load, before any other execution` – anson Oct 23 '12 at 00:34
  • More code would help to fully answer, but my answer to this old question might be useful in general - http://stackoverflow.com/questions/4329806/simple-php-login-with-cookie – El Yobo Oct 23 '12 at 00:36
  • One simple thing that I don't see, use `session_regenerate_id()` after logging authenticating the user to avoid session fixation attacks - http://shiflett.org/articles/session-fixation has a thorough explanation. – El Yobo Oct 23 '12 at 00:38
  • Your code is okay although kinda long. Why not just directly compare `if($_SESSION['signin'] == 5)` . Seem more straightforward to me. – William The Dev Oct 23 '12 at 00:39
  • I should add to be clear, *do not* rely on a generic *session or cookie* value like *5* or *true*, make it *user specific* – anson Oct 23 '12 at 00:40
  • andbeyond, I'd be interested in why a value stored *in the session* would need to be user specific; the session already *is* user specific, so something like setting loggedin to true would be fine. Any links or something to explain? – El Yobo Oct 23 '12 at 00:42
  • @ElYobo why would you ever rely on a constant (specifically a single digit number, but thats beside the point) to indicate whether a user is signed in or not? You shouldn't trust anything to be secure if you can help it, and in this case all it takes is literally using one or two built in functions to do all the work (as well as your advice for using session_regenerate_id() – anson Oct 23 '12 at 02:29
  • Because all you need is a boolean, as you're indicating a boolean status; are they logged in or not. As far as I can tell, if an attacker has somehow compromised your session data, then it doesn't matter what you put in there because they get at it anyway (although you've probably got bigger problems then). However, if there's a thorough explanation out there (e.g. Chris Shiftlett's on session fixation) I'd be interested to see how this could be a vulnerability. – El Yobo Oct 23 '12 at 03:40

2 Answers2

2

Very broad question. To answer in simple terms, yes, that's the usual approach. It's usually the things like following that needs to be made secure

  1. Is your code SQL injection safe? Can someone bypass the login?
  2. Can someone use a bruteforce attack?
  3. Is your code XSS safe?
  4. Is your code CSRF safe?
  5. Is the session cookie stored securely? Do you need to use HTTPS only or secure properties
  6. Is it safe against rainbow-table attacks? Are you using salting when storing passwords?

List can go on and of course these depends on the level of security needed.

Also, if the page is user specific, you also need to verify each action can be performed on the given object. For example if it's an order, is the order owned by the current logged in user, not just if a user is logged in.

Hope this helps.

nickhar
  • 19,981
  • 12
  • 60
  • 73
xelber
  • 4,197
  • 3
  • 25
  • 33
1

Broadly, yes, but without understanding more of your code it's hard to say.

You'd be better off assigning a unique value to the user - rather than an INT value of 5.

nickhar
  • 19,981
  • 12
  • 60
  • 73