TROUBLE WITH LOGIN SCRIPTING USING PHP & MYSQL

[thminco]

This is my first attemp at a log in system for a website.  Everything seems to work fine until the "successful" IF function near the end.  All I get it an output of "?>" instead of a redirect to the file "login_success.php".  Any help would be GREATLY appreciated!!  Tom

 

 

<?php

 

// Connect to server and select databse.

mysql_connect("localhost", "scripts3_public", "sfj123!")or die("cannot connect");

mysql_select_db("scripts3_sfj")or die("cannot select DB");

 

// username and password sent from form

$fusername=$_POST['fusername'];

$fpassword=$_POST['fpassword'];

 

// To protect MySQL injection (more detail about MySQL injection)

$fusername = stripslashes($fusername);

$fpassword = stripslashes($fpassword);

$fusername = mysql_real_escape_string($fusername);

$fpassword = mysql_real_escape_string($fpassword);

 

$sql="SELECT * FROM `users` WHERE `User name` = '$fusername' AND `Password` = '$fpassword'";

$result=mysql_query($sql);

 

if(!mysql_num_rows($result))

{echo "No results returned.";}

 

// Mysql_num_row is counting table row

$count=mysql_num_rows($result);

 

// If result matched $fusername and $fpassword, table row must be 1 row

 

if($count==1){

 

// Register $fusername, $fpassword and redirect to file "login_success.php"

session_register("fusername");

session_register("fpassword");

header("location:login_success.php");

}

 

else {

echo "Wrong Username or Password";

}

 

?>

[Drummin]

I don't see it but maybe you have it and are not showing it, but you should have session_start(); at the top of any page that will use sessions.

You are also missing $_ on both your session lines and you are not setting them to a value.  Should be.

 

// Register $fusername, $fpassword and redirect to file "login_success.php"
$_session['fusername']=$fusername;
$_session['fpassword]'=$fpassword;
header("location:login_success.php");
}

 

[ManiacDan]

1)  $_SESSION is in all caps. 

 

2)  Session_register (and related functions) are deprecated and should not be used.  Read the manual page on sessions for modern syntax

 

3)  Don't post your database password in public.

 

4)  Your use of stripslashes leads me to believe you're using magic_quotes.  This is also deprecated and should be removed immediately.

 

5)  you must die() immediately after header calls.

 

6)  There is a space after the colon in a header redirect, and Location is capitalized.

 

7)  View the SOURCE of a page to see the complete output.

 

  Always store passwords encrypted with a salted one-way hash in the database.  Never store them "plain" like this.

 

9)  This is clearly copied and pasted from a tutorial.  Stop using that tutorial right now, it appears to be 5-6 years old.

 

-Dan

[PFMaBiSmAd]

All I get it an output of "?>"

 

Are you sure php is installed on your server and you browsed to the URL of your page so that php code would be parsed/executed on the server?

[Drummin]

Man I hate when I make mistakes.  ManiacDan is of course correct on every point and my posted code should have been.

// Register $fusername, $fpassword and redirect to file "login_success.php"

$_SESSION['fusername']=$fusername;

$_SESSION['fpassword']=$fpassword;

header("location: login_success.php");

die();

}

[thminco]

OK, here is my updated code thanks to all of the wonderful corrections (Thank you all SO MUCH!!)  The script is still not doing the re-direct for some reason??  Yes, I was indeed getting help from an (old) online tutorial as I am very new to the log in work.  Dan...I don't even know what magic quotes are??  Should I still not use the stripslashes??  Also included below is the landing file code.

 

if($count==1){

 

// Register $fusername, $fpassword and redirect to file "login_success.php"

$_SESSION["fusername"]=$fusername;

$_SESSION["fpassword"]=$password;

header("Location: login_success.php");

die();

 

}

 

else {

echo "Wrong Username or Password";

}

 

ob_end_flush();

 

?>

 

 

 

LANDING FILE CODE:

 

<?

session_start();

if(!session_is_registered(fusername)){

header("location:sfjogin.php");

}

?>

 

<html>

<body>

Login Successful

</body>

</html>

[Far Cry]

OK, here is my updated code thanks to all of the wonderful corrections (Thank you all SO MUCH!!)  The script is still not doing the re-direct for some reason??  Yes, I was indeed getting help from an (old) online tutorial as I am very new to the log in work.  Dan...I don't even know what magic quotes are??  Should I still not use the stripslashes??  Also included below is the landing file code.

<?

if($count==1){

 

// Register $fusername, $fpassword and redirect to file "login_success.php"

$_SESSION["fusername"]=$fusername;

$_SESSION["fpassword"]=$password;

header("Location: login_success.php");

die();

 

}

 

else {

echo "Wrong Username or Password";

}

 

ob_end_flush();

 

?>

 

 

 

LANDING FILE CODE:

 

<?

session_start();

if(!session_is_registered(fusername)){

header("location:sfjogin.php");

}

?>

 

<html>

<body>

Login Successful

</body>

</html>

 

1). Magic qoutes was used to escape strings to prevent MySQL injection, just use mysql_real_escape_string();.

2).

<?
session_start();
if(!session_is_registered(fusername)){
header("location:sfjogin.php");
}
?>

Should be...

<?php
session_start();
if(!$_SESSION['fusername']){
header("Location:sfjogin.php");
die();
}
?>

3). Never use shorthand (<? ?>) PHP. Bad practice.

[ManiacDan]

1)  Magic quotes is a php.ini setting that randomly sticks backslashes into your strings if they have single quotes or other special characters.  It's a VERY old method of SORT OF protecting against SQL injection.  If your php.ini has it on, turn it off.

 

2)  Your login page snippet is now correct, though there's no reason to be using the output buffering code probably.  ob_end_clean is not usually necessary, but I don't know what the rest of your code is using.

 

3)  Your landing page suffers from the same problems as your first page did.  session_is_registered(fusername) should be replaced with isset( $_SESSION['fusername'] ).  Die after a header.  Etc.

 

4)  Still, view your source to see the full output.  It's possible your server doesn't even know what PHP is and all of this is a wasted exercise.

[thminco]

Thanks again everyone.  I feel like I should be paying you all for this help!!  I corrected all the landing file errors but don't think I am even getting to that page (file) because the url doesn't change in my browser address box.  I have 5 sites that are all using php flawlessly (with I'm sure lots of newbie coding flaws) at my host.  (They are using php5.2... something)  I had some little echo statements in the if execution at the end of my code to make sure I was getting that far and the echos worked fine, so I'm pretty sure its not an exersize in futility.

 

UPDATED CODE FROM INITIAL SCRIPT...

 

if($count==1){

 

// Register $fusername, $fpassword and redirect to file "login_success.php"

$_SESSION['fusername']=$fusername;

$_SESSION['fpassword']=$fpassword;

header('Location: http://www.***.com/login_success.php');

die();

 

}

 

else {

echo "Wrong Username or Password";

}

 

?>

 

 

 

UPDATED CODE FROM LANDING FILE PAGE (login_success.php)...

 

<?php

session_start();

if(!$_SESSION['fusername']){

header("Location: sfjlogin.php");

die();

}

?>

 

<html>

<body>

Login Successful

</body>

</html>

 

[thminco]

BTW Dan, when I view source from the browser at sfjlogin.php I don't see anything (because it is all php on the server?)  I don't get redirected anywhere from there.  Thanks very much again!!!  Tom

[Drummin]

Can you echo $count to see if that value is as expected and the enclosed code is being executed?

[thminco]

Yes, if I add the 2 echo statements below, I get an output as follows:

 

1thomas

 

This is what I would expect and shows that I have made it into the "IF" function successfully.

 

 

if($count==1){

echo $count;

echo $fusername;

// Register $fusername, $fpassword and redirect to file "login_success.php"

$_SESSION['fusername']=$fusername;

$_SESSION['fpassword']=$fpassword;

header('Location: http://www.***.com/login_success.php');

die();

 

}

 

else {

echo "Wrong Username or Password";

}

 

 

?>

[Drummin]

I usually quote my header location.

header("Location: http://www.***.com/login_success.php");

[thminco]

Thanks drummin.  I changed to the double quotes but get the same result...an output of the $fusername and $count variables but no url redirection.

[xyph]

Single quotes are better because it saves PHP from parsing a string it doesn't have to.

 

To the OP, you should be programming with errors visible. You're getting a header error that isn't being displayed.

 

Check out header in the manual. You can't output before calling it.

[thminco]

Sorry gang, I double posted!  Man, it hurts being such a newbie!

 

[thminco]

Thanks xyph...I put that code in and got this error message:

 

Warning: session_start() [function.session-start]: Cannot send session cache limiter - headers already sent (output started at /home2/scripts3/public_html/sfjlogin.php:2) in /home2/scripts3/public_html/sfjlogin.php on line 7

1thomas

Warning: Cannot modify header information - headers already sent by (output started at /home2/scripts3/public_html/sfjlogin.php:2) in /home2/scripts3/public_html/sfjlogin.php on line 43

 

It seems I have something wrong with the way I am using "session_start()" on line 7??

 

I am headed back to the manual, but unfortunately it is a little bit greek for me!  Thanks to everyone for the help!!  BTW...thanks Dan for the tip on using encryption!!  Very cool!!  Also, it seems like I really don't need the escaping for sql injection since I am not inserting or appending to the sql database in this script??  Here is my entire code at this point:

 

<?php

 

ini_set('display_errors',1);

error_reporting(-1);

 

session_start();

 

// IMPORT FORM VARIABLES

$fusername=$_POST['fusername'];

$fpassword=$_POST['fpassword'];

 

// CONNECT TO SERVER AND SELECT DATABASE

mysql_connect("localhost", "scripts3_public", "******")or die("cannot connect");

mysql_select_db("scripts3_sfj")or die("cannot select DB");

 

// PROTECT AGAINST MYSQL INJECTION

mysql_real_escape_string($fusername);

mysql_real_escape_string($fpassword);

 

// ENCRYPT FORM PASSWORD TO COMPARE WITH DATABASE ENCRYPTED PASSWORD

$encrypt_fpassword=md5($fpassword);

 

// LOOKUP USERNAME AND PASSWORD IN DATABASE COMPARED TO FORM ENTRIES

$sql="SELECT * FROM `users` WHERE `User name` = '$fusername' AND `Password` = '$encrypt_fpassword'";

$result=mysql_query($sql);

 

if(!mysql_num_rows($result))

{echo "No results returned.";}

 

// COUNT RESULTS

$count=mysql_num_rows($result);

 

// IF COUNT IS 1,REGISTER USERNAME AND PASSWORD AND REDIRECT

 

if($count==1){

 

echo $count;

echo $fusername;

 

$_SESSION['fusername']=$fusername;

$_SESSION['fpassword']=$fpassword;

header("Location: http://www.******.com/login_success.php");

die();

 

}

 

else {

echo "Wrong Username or Password";

}

 

?>

[xyph]

Even SELECTing you should prevent injection.

 

Imagine a query like

SELECT * FROM `users` WHERE `user`='username' AND `pass`='password' OR 1=1

 

session_start() and header() must be called before any output to the browser.

[litebearer]

my 2 cents...

1) if you haven't previously stored the password in a hashed form, then hashing the password for use in the query will not work

 

2) this

echo $count;
echo $fusername;

$_SESSION['fusername']=$fusername;
$_SESSION['fpassword']=$fpassword;
header("Location: http://www.******.com/login_success.php");
die();

will result in a header issued problem

[Drummin]

Obviously my post aren't worth much but get rid of the echo lines sending something to the browser or at least move them to after setting the session values, but as you're redirecting you don't need them.

if($count==1){

$_SESSION['fusername']=$fusername;
$_SESSION['fpassword']=$fpassword;
header("Location: http://www.******.com/login_success.php");
die();

}

[thminco]

I can't figure out what this means:

 

"session_start() and header() must be called before any output to the browser."

 

What is meant by output to the browser?  The calling of the header function?

 

I do call "session_start();" at the start of my code and per the manual am supposed to also call "session_name" before "session_start" and before any other code, thus I now have my code beginning as follows:

 

<?php

 

session_name("sfjloginsession");

session_start();

 

// TURN ON ERROR REPORTING

ini_set('display_errors',1);

error_reporting(-1);

 

 

 

My latest error gives this:

 

Warning: Cannot modify header information - headers already sent by (output started at /home2/scripts3/public_html/sfjlogin.php:2) in /home2/scripts3/public_html/sfjlogin.php on line 46

 

Is this maybe because I have some session info already stored and need to clear it??

[Drummin]

session_start(); needs to be before using or calling any session and before anything is sent to the browser. Assuming "sfjloginsession" is supposed to be a session value.

<?PHP
session_start();
$sfjloginsession=$_SESSION['sfjloginsession'];

// TURN ON ERROR REPORTING
ini_set('display_errors',1);
error_reporting(-1);

[ManiacDan]

Echo, print, printf, and various other output functions count as "output."  You also cannot have anything outside of PHP tags. If your page begins with a single space before the <?php tag, then it will fail to use a header.  Similarly, if you include a file with a single space after the closing ?>, it will fail.

 

The error messages you're pasting contain a file and line number.  That's VERY easy to figure out.  Go to that file.  Go to that line.  Fix that.  You cannot output anything on any page if you're going to be using a header redirect.

 

You're using md5 on your passwords now.  2 things:

1)  use sha1, it's better

2)  You need to update your database so the passwords in the DB are also hashed the same way. 

 

 

[thminco]

DAN!!!!!!!!!!  You are truly the man!  There was one blank line in the code before the opening php (<?php) statement in line 2.  Once I deleted that first blank line...It worked just fine!  Thank you so much for all your help, and BTW I will look into the other encrypting method you mentioned.  I have only 2 passwords in the database (one of them encrypted/hashed) so now is a good time to start using whatever is the best encryption!!  Thanks again SO MUCH to everyone who helped!  I am sure my code is overall a LOT cleaner and better, but with plenty still to do!!  Tom

[ManiacDan]

If you're starting from scratch, use sha1() with a salt.  Your encrypted password should be sha1($password . 'someReallyLongRandomStringYouComeUpWithYourself').  Keep the random string secret.  It prevents your code from being vulnerable to rainbow table attacks.