Page 1 of 1

Does this login code look secure?

Posted: Mon Feb 03, 2003 6:06 pm
by icesolid

Code: Select all

<?php
if($_POST["submit"]) {
$link = mysql_connect("localhost", "username", "xxxx") or die("Could not connect");
mysql_select_db("users") or die("Could not select database");

$username = strip_tags($_POST["username"]);
$password = strip_tags($_POST["password"]);

$sql = "SELECT username, password FROM general_access WHERE username='&username' && password='$password'";
$result = mysql_query($sql);
$row = mysql_fetch_array($result);

	if(!$username) {
	$error = "1";
	header("Location: login.php?error=$error");
	} elseif(!$password) {
	$error = "2";
	header("Location: login.php?error=$error");
	} elseif(!$username == $row["username"] && !$password == $row["password"]) {
	$error = "3";
	header("Location: login.php?error=$error");
	} elseif($username == $row["username"] && $password == $row["password"]) {
	header("Location: secure.php");
	}

mysql_free_result($result);
mysql_close($link);
} else {
?>
<b>Enter you user information:</b>
<br><br>
<form method=post action="login.php">
Username: <input type=text name="username"><br>
Password: <input type=text name="password">
<br><br>
<input type=submit name="submit" value="Log In"> &nbsp;<input type=reset value="Reset">
</form>
<?php
	echo "<font color="#FF0000">\n";
		if($_GET["error"] == "1") {
		echo "No username entered.\n";
		} elseif($_GET["error"] == "2") {
		echo "No password entered.\n";
		} elseif($_GET["error"] == "3") {
		echo "Invalid username or password entered.\n";
		}
	echo "</font>\n";
}
?>

Posted: Mon Feb 03, 2003 6:58 pm
by volka
a typo at
username='&username'
better use mysql_escape_string than strip_tags.
It's better to test the existence (and length) of username and password before querying mysql. If you do so and records are returned, e.g. tested by mysql_num_rows*, WHERE username='$username' AND password='$password' already asured that login/password were correct, so there's no need to check equality again with php.
header("Location: secure.php");
what happens if someone requests this page directly without passing the login?

*: you may also use

Code: Select all

$sql = "SELECT count(*) FROM general_access WHERE username='$username' AND password='$password'";
$result = mysql_query(...) ...
// if the number of records matching username and password is 1  that login was successful
$loggedIn = (array_shift(mysql_fetch_row($result)) ==1);

Posted: Tue Feb 04, 2003 12:02 am
by bionicdonkey
the passwords should be encrypted and db connection stuff should be stored in an external file