HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

For a login portal, what security measures are needed to prevent unauthorized access?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
preventneededwhatareloginunauthorizedsecurityforportalmeasures

Problem

I'm designing a login portal that has one angularjs page that displays/processes data queried from a database. I'm relying on a few php pages (a loginpage.php [verifies credentials/loads session variables], a check.php [page that is included in all php pages, verifies session variables are 'okay'], and a getter.php [queries database/returns json] page) to do handle the getting/setting of data for my angularjs SPA.

I did a lot of reading into session variables, security, sql-injection, etc. to prevent users from gaining access to additional unauthorized data from my database.

While I know it is hard to say that methods are 100% secure, I was wondering if someone could give me some insight into whether or not there are any severe gaps/holes in my session security logic.

login.php (abbreviating code, obv session_start is in the beginning of all php pages)

after verifying credentials, the following session variables are set:

//example of (directly queried/set, Trusted) - I directly query a login db,          
//which returns a data set of fields. I take the $response and directly set
//the session variables like so: $_SESSION['useremail']= $query->record[0]->Email;

$_SESSION['user'] = $username;
$_SESSION['useracctid'] = (directly queried/set, Trusted)
$_SESSION['userisauthenticated'] = true;
$_SESSION['timeactive'] = date("Y-m-d H:i:s", strtotime("+20 minutes"));
$_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
$_SESSION['count'] = 5;


check.php



getter.php (abrv. code, verifies session credentials, then queries db and returns data for angularjs SPA) The angularjsview.php page passes the $_SESSION['useracctid'] as a ng-init to the controller, and the controller posts (via angularjs' $http) the useracctid to the getter.php page.



With my security logic, can I 'assume' that even if a malicious user authenticates, he/she cannot get more information than the jeopardized user account? Am I missing a security check or have any huge exploits to

Solution

Your session logic seems good to me.

Prepared Statements

Use prepared statements.

It's unclear where $_SESSION['useracctid'] comes from, but it is placed directly inside the query, which is a bad idea. It might have been cleaned previously (currently, it's cleaned indirectly, via substr and clean on $useracctid, but you didn't give that function), or it might not have, or it currently is, but your code changes, and it's not cleaned in the future.

Or, as it's stored in a function, it is currently cleaned by the caller, but a different script also needs this function and doesn't clean it. You just don't know, and prepared statements are the recommended solution to prevent SQL injection, so they should be used.

Misc

  • use session_regenerate_id(true); to not only change the session id, but to destroy the previous session file.



  • It's good to regenerate the session id periodically, but you should also always call session_regenerate_id(true); when the session state changes, eg when logging in (less important today, as use_trans_sid is set to 0 by default, and thus session fixation is less of a problem, but still).



  • binding the session to the IP is a pretty good security mechanism, but it could possibly inconvenience users whose IP changes often for legitimate reasons (eg using a cellphone). A less secure, but more user oriented approach might be to bind it to the user agent.



  • call checksession() directly in check.php. There doesn't seem to be a situation when it will not be called, and when including check.php, it is easy to forget that call.



  • is exit needed after header?: Yes, it is. see also here for consequences of not doing it (although it's mostly a lesson about following REST and having CSRF protection)

Context

StackExchange Code Review Q#85743, answer score: 4

Revisions (0)

No revisions yet.