0

Recently I tried to create a login webpage for my project. So there's the index.html, the login page where the person has to login. It sends the query to checklogin.php which is this:

<?php
ob_start();
$host="localhost"; // Host name 
$username=""; // Mysql username 
$password=""; // Mysql password 
$db_name="test"; // Database name 
$tbl_name="members"; // Table name 

// Connect to server and select databse.
mysql_connect("wordshare.zxq.net", "754319_guest", "guest")or die("cannot connect"); 
mysql_select_db("wordshare_zxq_users")or die("cannot select DB");

// Define $myusername and $mypassword 
$myusername=$_POST['myusername']; 
$mypassword=$_POST['mypassword']; 

// To protect MySQL injection (more detail about MySQL injection)
$myusername = stripslashes($myusername);
$mypassword = stripslashes($mypassword);
$myusername = mysql_real_escape_string($myusername);
$mypassword = mysql_real_escape_string($mypassword);
$sql="SELECT * FROM members WHERE username='$myusername' and password='$mypassword'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);

// If result matched $myusername and $mypassword, table row must be 1 row
if($count==1){

// Register $myusername, $mypassword and redirect to file "login_success.php"
session_start();
session_register("myusername");
session_register("mypassword"); 
header("location:main.php");
}
else {
    header("location:index.html");
}
ob_end_flush();
?>

Then the main page checks for the session, and if you are not logged in it redirects you to index.html and the code is as following:

<?php
// Connects to your Database
mysql_connect("wordshare.zxq.net", "754319_guest", "guest")or die("cannot connect");
mysql_select_db("wordshare_zxq_users")or die("cannot select DB");
//checks cookies to make sure they are logged in
session_start();
session_start();
if(!session_is_registered(myusername)){
header("location:index.html");
}
?>

The problem is that I don't get redirected back to the main page if I go directly to the main.html. I've tried using echo to find out whether the session is recorded, and it returned undefined. What really baffled me was that even when I try to print out something else like a word, it also returns me "undefined". Can anyone help me?

yAnTar
  • 4,269
  • 9
  • 47
  • 73
Dae Koon Lim
  • 5
  • 1
  • 2
  • session is starting twice in the second code snippet, if(isset($_SESSION["myusername"])) to check if username is set.. – Gntem Jul 21 '12 at 09:49

3 Answers3

4

You have many issues with your code. Wherever you copied that over, it is a very bad example. I have some spare time, so I highlight what I can see, most serious first:

  • You store plain-text passwords in your database. This is very serious, because in case someone will hack your database, the username and password information can be retrieved easily. This is a very common mistake but a very bad one. Instead hash your passwords, for example with the phpass library. That website explains very well what this is about. So if you want to learn, that site does not only have code but gives also a quite good and generic description.

  • The code expects that get_magic_quotes_gpc is enabled. Instead it should refuse to work if it is enabled. Assuming that magic quotes are enabled is a security issue because it prevents you from writing safe code

    Your Code:

    $mypassword=$_POST['mypassword'];

// To protect MySQL injection (more detail about MySQL injection)
$myusername = stripslashes($myusername);

    Suggestion:

    if (get_magic_quotes_gpc()) {
    throw new UnexpectedValueException('get_magic_quotes_gpc must be off.');
}
$mypassword = $_POST['mypassword'];

    (There is no need to do stripslashes any longer)

  • You use other outdated language features. This is merely a sympton that from whereever you have copied over that code, you do not have taken code that is state of the art. A few issues you have:

    • mysql_* functions. Use PDO instead. It's much more simple to use and much more powerful. It helps you pro-actively to prevent SQL injection by providing so called prepared statements (also called parameterized queries). Learn about it, use it.
    • session_register and session_is_registered functions. Those were used to register global variables inside the session. They are unsafe and deprecated. Instead use the $_SESSION superglobal, like you use the $_POST superglobal already.

  • One long line-up of code. You do not make use of subroutines. Albeit they could be very helpful for you. Learn how you can write your own functions because you can do programming like writing a text.

Example:

<?php
require('my-functions.php');

$location = 'index.html';

if ($user = user_form_submitted() && user_is_valid($user)) {
    user_login_into_session_($user);
    $location = 'main.php';       
}

redirect($location);
?>

As you can see this is quite easy to read. Just then define functions that do the job, e.g.:

/**
 * redirect request
 *
 * @param string $location
 */
function redirect($location) {
    if (!headers_sent()) {
        header("Location: " . $location);
    }
    printf('Moved <a href="%1$s">here</a>.', htmlspecialchars($location));
}

That's just one example. So you can start to program already without even thinking how all the glory different details will work.

Related answers about using and extracting functions and using PDO:

Community
  • 1
  • 1
hakre
  • 193,403
  • 52
  • 435
  • 836
  • Thanks for the tips. Now I've changed the session code to this : $_SESSION['myusername'] = $myusername; $_SESSION['mypassword'] = $mypassword; and in the main.php I've set the session checking code to this: if (!isset($_SESSION['myusername'])){ header("location:index.html"); } However it seems like the session isn't set and I keep getting redirected back to the main page. Any idea whats wrong with the webpage? – Dae Koon Lim Jul 25 '12 at 16:34
  • Adding in, security isn't my concern because I'm just making this website for practice and I'm not going to do anything serious with it. But still, thanks for your suggestion. I'll use it when the need arises. – Dae Koon Lim Jul 25 '12 at 16:41
  • Security is not only important in the sense of attacking and the like, but also in the sense to ensure the application is working as intended. If the session does not appear to be set you need to debug that more specifically. And probably the session class outlined here might be insightful: http://stackoverflow.com/questions/11596082/php-session-class-similar-to-codeigniter-session-class-exists/11596538#11596538 – hakre Jul 25 '12 at 21:09
  • Whats really weird is that the code works if I use the session_register, but doesn't when I use $session. – Dae Koon Lim Jul 26 '12 at 00:44
1

I think you missed a quote on this line: if (!session_is_registered(myusername)) {

it should be: if( !session_is_registered("myusername")) {

Timo Tijhof
  • 10,032
  • 6
  • 34
  • 48
sendi
  • 11
  • 1
0

You have two sessions (with the same name), which means the second overwrites the first session.

Why are you using session_register when the manual says

This function has been DEPRECATED as of PHP 5.3.0 and REMOVED as of PHP 5.4.0.

Instead:

Type $_SESSION['myusername'] = $usr; and $_SESSION['mypassword'] = $pswd;.

Remove the second session_start() in main.php.

Type if (isset($_SESSION['myusername']) instead of session_is_registered().

Nadav S.
  • 2,429
  • 2
  • 24
  • 38