3
\$\begingroup\$

I am hoping that someone can help me out here. I am creating a PHP web app that requires a user to login to view their data in the MySQL database.

I just want to know if this is a good approach and is it secure enough. I know that nothing is secure in PHP scripting but I need this fairly secure.

Here is what I am doing step by step. If you could point me in the right direction and tell me what is wrong that would be great!

  1. User logs in with user and password.
  2. PHP checks user and password against the database.
  3. If OK, I store the $SESSION["username"] (their username) to the session.

On every request/page load I have a function (below) to check the session:

function check_session_valid(){ if(!isset($_SESSION['username'])||$_SESSION['username'] == ""){ header("Location: /login.php"); } }

Now is this safe or am I doing this all completely wrong?

Here is what I am thinking that the user could do to "hack" the app:

On the registration page, when the user enters there username it will let them know if the username is available.

Could the "hacker" not just find a username that is not available and then write a PHP script to set the $_SESSION["username"] in their browser, then navigate to the .index.php page and be logged into some other users account?

I am also using session_start() before I check_session_valid() on every request. Is that wrong?

\$\endgroup\$

closed as off-topic by Brythan, Hosch250, TheCoffeeCup, syb0rg, vnp Dec 25 '14 at 7:15

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must include the code to be reviewed. Links to code hosted on third-party sites are permissible, but the most relevant excerpts must be embedded in the question itself." – Brythan, Hosch250, TheCoffeeCup, syb0rg, vnp
If this question can be reworded to fit the rules in the help center, please edit the question.

1
\$\begingroup\$

One long line that is hard to read.

The technique looks legitimate, but I caution against reusing 'username' as the key for storing the login state. Who knows, someday you might want to be able to persist a username even when logged out. Or maybe a multi-page user registration form would set $_SESSION['username'] between pages. I recommend choosing something more explicit, such as 'login_time' — and you could also use it later to help with automatic logouts after a certain time.

Note that calling header(…) does not halt the regular program flow. Whoever calls check_session_valid() will still need to terminate further processing if the session is invalid, so your check_session_valid() should return a TRUE/FALSE value as well. You could also throw an exception to ensure that regular program flow is impossible.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.