0

For my custom framework I let users log in and set a session as follows:

<?PHP
session_start();

// bunch of code

if (isset($_SESSION['id') {
    // check time and regenerate session id every 10 minutes
    // session_regenerate_id(true);
}

// some more code

if (isset($_POST['login']) {
    // check if login is valid, when it is:
    $_SESSION['user_id']       = getUserData('id');
    $_SESSION['user_name']     = getUserData('name');
    $_SESSION['user_is_admin'] = getUserData('admin'); // filled with 0 or 1
}

Everything is stored in a database with the passwords hashed in BCRYPT. On top of this I force SSL so users can't reach the website through ordinary http.

Is this method safe? If not; what are the security flaws and what can I do to make this more secure?

Harrie
  • 69
  • 9

2 Answers2

1

First of all, to get the session ID you should use session_id(), $_SESSION['id'] will throw an undefined offset exception, unless you put something under that value.

You shouldn't use session ID to evaluate if the user is authenticated or not because that will always evaluate to TRUE. Session ID will always be present when you start a session, you should store your user in the session after the successful authentication e.g. $_SESSION['auth_user'] = get_user_from_db() and then use that to evaluate whether the user is authenticated:

if (isset($_SESSION['auth_user'])) {
    // Your logic
}

Read this post for authentication best practices.

In a nutshell, your session cookies should only be accessible via http, you should, as you did, force only secure cookies. You should regenerate the session id every time your user gets privilege (e.g. after successful authentication).

You need to implement CSRF protection mechanism.

You should implement login throttling (timeout after user has X number of failed login attempts in a certain timeframe e.g. 1 minute) to protect against brute force attacks.

Sasa Blagojevic
  • 1,692
  • 13
  • 20
0

The best approach would always be to at the login screen and immediately post login to force a new session id generated using random numbers.

session_start();
$newsessid = somerandomnumberfunction();
session_id($newsessid);

you can also use session_regenerate_id() function to generate a new id

session_start();
session_regenerate_id();

Also its always good to ensure every valid session is checked against an ip. One good method is to store the session id and remote ip information in a table, or better store the ip as a session variable itself, once the user logs in and ensure that this is continued for remaining pages for security. This ofcourse wont work when users use the same office or shared network as the ip to the outside world is the same.

https is always a good idea for sensitive sites, but keeping it persistent for all pages which use session is important if you really want a foolproof system else anyone can always sniff your packets.

P.S your method is safe according to the best of my knowledge.

Few Tips:

  • Make sure you always use a new self generated session id on a successful login attempt.
  • Try setting the session.use_only_cookies = 1 and check if all works fine.
  • Use https always throughout to ensure no one can sniff your session id.
  • Store session id, remote IP information and compare for successive pages
  • set session.use_trans_sid = 0 in /etc/php5/apache2/php.ini file
Asfandyar Khan
  • 1,386
  • 13
  • 31
  • Storing the remote IP is ok but you shouldn't validate the session against the IP, if the user is behind a proxy (which a lot of users on mobile devices are) their IP could change during a session which will kick them out. Also say you logged into the site on your laptop at the office, went home and the went back to that website, you would be forced to sign in again. – Matt Rink Aug 30 '17 at 10:39
  • @MattRink Then i think the best approach is to use a cookie. it will allow to connect from arbitrary IP, on an arbitrary medium - at which point. – Asfandyar Khan Aug 30 '17 at 10:50
  • @AsfandyarKhan thank you for your answer, why should I use a wel generated session id instead of session_regenerate_id()? – Harrie Aug 30 '17 at 11:09
  • @AsfandyarKhan sorry I misunderstood then, I thought you were inclining a _self_ generated id would be better than session_regenerate_id() – Harrie Aug 30 '17 at 11:16