Security

[genics]

Hi guys,

I've created an add user script, however I was to stop people from adding two identical usernames. What's the best way to make sure each one is unique?

Here's my script:


<?php

if(isset($_POST['add']))
{
$user_name = $_POST['username'];
$user_pass = md5($_POST['password']);
$user_email = $_POST['email'];
$user_ip = $_SERVER['REMOTE_ADDR'];
$user_access = $_POST['access'];

$addUser =
"INSERT INTO clanms_users (
user_name,
user_pass,
user_email,
user_ip,
user_access
) VALUES (
'$user_name',
'$user_pass',
'$user_email',
'$user_ip',
'$user_access'
)";

mysql_query($addUser) or die('<p>User Not Added.</p>');
print '<p>User Added.</p>';
}

?>

[aj_nsc]

set the user_name field to "unique" in the database.

[tca]

<?php

if(isset($_POST['add']))
{
$user_name = $_POST['username'];
$user_pass = md5($_POST['password']);
$user_email = $_POST['email'];
$user_ip = $_SERVER['REMOTE_ADDR'];
$user_access = $_POST['access'];

//Check if user_name is available
$query = "SELECT id FROM clanms_users WHERE user_name='$user_name'";
$result = @mysql_query ($query); //Run the query
if (mysql_num_rows($result) == 0) { //The username is available
//Add the user

$addUser =
"INSERT INTO clanms_users (
user_name,
user_pass,
user_email,
user_ip,
user_access
) VALUES (
'$user_name',
'$user_pass',
'$user_email',
'$user_ip',
'$user_access'
)";

mysql_query($addUser) or die('<p>User Not Added.</p>');
print '<p>User Added.</p>';
}
} else {
echo 'That User Name is already taken';
}
?>

[genics]

Legend. Cheers mate - will try it out.

[genics]

Hmm, I'm getting an error here:

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in

[aj_nsc]

Put in


or die(mysql_error());


after your query and tell us what it spits out.

It may just be the space between @mysql_query ($query);

[genics]

Ok, I've fixed the problem, he's the final code:


<?php

if(isset($_POST['add']))
{
$user_name = $_POST['username'];
$user_pass = md5($_POST['password']);
$user_email = $_POST['email'];
$user_ip = $_SERVER['REMOTE_ADDR'];
$user_access = $_POST['access'];

$query = "SELECT user_id FROM clanms_users WHERE user_name='$user_name'";
$result = @mysql_query ($query) or die(mysql_error()); ;
if(mysql_num_rows($result) == 0) {

$addUser =
"INSERT INTO clanms_users (
user_name,
user_pass,
user_email,
user_ip,
user_access
) VALUES (
'$user_name',
'$user_pass',
'$user_email',
'$user_ip',
'$user_access'
)";

mysql_query($addUser) or die('<p>User Not Added.</p>');
print '<p>User Added.</p>';
}
else {
echo 'That Username Is Already Taken';
}
}

?>

[tca]

Sorry 'bout that. I should have commented to use your primary key for id in the query.

Glad you got it going.

TC

[genics]

Thanks for the help mate :)

[Michaelttkk]

here's what I tried for the login script


<?
if (isset($_GET['username']) && !empty($_GET['username'])) $id = addslashes($_GET['username']);

$query = "SELECT * FROM members where '$username'= '{$user_name}'";
$result = mysql_query ($query, $connect);

?>


it doesn't seem to work and any help on the password too?

[DARTHTAMPON]

Are you sure you do not want to use $_POST instead of $_GET? How are you passing the data from the form? Usually when dealing with logins you want to post the data to hide it from view.

[bokeh]

This code allows collisions. Use the unique column as suggested in post #2.

[genics]

Here's my login script if you fancy taking a butchers.


<?php

session_start();
if (isset($_POST['username']) && isset($_POST['password'])) {
include('lib/db_connect.php');
$error = '';
$username = $_POST['username'];
$password = md5($_POST['password']);

$sql = "SELECT * FROM users WHERE user_name = '$username' AND user_pass = '$password'";
$result = mysql_query($sql);

if(mysql_num_rows($result)!= 1) {
$error = "Login Incorrect";
} else {
$_SESSION['user_name'] = "$username";
$_SESSION['user_ip'] = $_SERVER['REMOTE_ADDR'];
$_SESSION['logged_in'] = true;
header('Location: index.php');
}
include('lib/db_disconnect.php');
}

?>

[genics]

I've just realised my code posted above is open to SQL injection attacks, do any PHP gurus know how I can prevent the attacks?

:)

[DARTHTAMPON]

you could make sure the referer is = what you want it to be and/or add a hidden field which requires validation on the php side.

[aj_nsc]

how does that stop an injection attack?

see this (http://www.netlobo.com/preventing_mysql_injection.html)

[genics]

Wouldn't it be simpler with something along the lines of


mysql_real_escape_string()

[genics]

Whoops missed your post aj_nsc - I'm checking that out now. :)

[genics]

Ok, here's my updated code:


session_start();
if (isset($_POST['username']) && isset($_POST['password'])) {
include('lib/db_connect.php');
$error = '0';
$username = $_POST['username'];
$password = md5($_POST['password']);

function quote_smart($username)
{
if (get_magic_quotes_gpc()) {
$username = stripslashes($username);
}
if (!is_numeric($username)) {
$username = "'" . mysql_real_escape_string($username) . "'";
}
return $username;
}

$sql = "SELECT * FROM users WHERE user_name = '$username' AND user_pass = '$password'";
$result = mysql_query($sql);

if(mysql_num_rows($result)!= 1) {
$error = "Login Incorrect";
} else {
$_SESSION['user_name'] = "$username";
$_SESSION['user_ip'] = $_SERVER['REMOTE_ADDR'];
$_SESSION['logged_in'] = true;
header('Location: index.php');
}
include('lib/db_disconnect.php');
}


Do you believe this is secure enough?

[aj_nsc]

Your code, as written, is no different than before. You didn't call the function that you stuck in there...

replace this:

$username = $_POST['username'];


with this:

$username = quote_smart($_POST['username']);


Also, I think it's good practice to impose a limit on all applicable queries. i.e. in this case, I would write limit 1 at the end of your query.

[genics]

Whoops, yep c+p'd over my function call.

Here's my updated code:


session_start();
if (isset($_POST['username']) && isset($_POST['password'])) {
include('lib/db_connect.php');

function quote_smart($username)
{
if (get_magic_quotes_gpc()) {
$username = stripslashes($username);
}
if (!is_numeric($username)) {
$username = "'" . mysql_real_escape_string($username) . "'";
}
return $username;
}

$error = '0';
$username = quote_smart($_POST['username']); ;
$password = md5($_POST['password']);

$sql = "SELECT * FROM users WHERE user_name = '$username' AND user_pass = '$password' LIMIT 1";
$result = mysql_query($sql);

if(mysql_num_rows($result)!= 1) {
$error = "Login Incorrect";
} else {
$_SESSION['user_name'] = "$username";
$_SESSION['user_ip'] = $_SERVER['REMOTE_ADDR'];
$_SESSION['logged_in'] = true;
header('Location: index.php');
}
include('lib/db_disconnect.php');
}


I'm getting this error when the form is submit:

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource

[genics]

Ok I fixed the problem. It was a problem within the function doubling up the '$username' to ''$username'' in the query.

It's all sorted now - here is my final code:


session_start();
if (isset($_POST['username']) && isset($_POST['password'])) {
include('lib/db_connect.php');
$error = '0';

function quote_smart($username)
{
if (get_magic_quotes_gpc()) {
$username = stripslashes($username);
}
if (!is_numeric($username)) {
$username = mysql_real_escape_string($username);
}
return $username;
}

$username = quote_smart($_POST['username']);
$password = md5($_POST['password']);

$sql = "SELECT * FROM users WHERE user_name = '$username' AND user_pass = '$password' LIMIT 1";
$result = mysql_query($sql) or die(mysql_error());

if(mysql_num_rows($result)!= 1) {
$error = "Login Incorrect";
} else {
$_SESSION['user_name'] = "$username";
$_SESSION['user_ip'] = $_SERVER['REMOTE_ADDR'];
$_SESSION['logged_in'] = true;
header('Location: index.php');
}
include('lib/db_disconnect.php');
}


Feel free to pick it apart :)

[sitehatchery]

You don't need or want to use $_GET. Also, validate it.

for instance,

if(cype_alnum($_POST['username'])) $clean['user']=$_POST['username'];

It just validates $_POST['username'] as alphanumeric.

Then, user $clean['username'] throughout your script instead of $_POST['username'].

[edwardinnz]

Hi guys

I've done some code like this

$theFirstName = $_POST["theFirstName"];
$theLastName = $_POST["theLastName"];
$theUsername = $_POST["theUsername"];
$thePasswordm = $_POST["thePasswordm"];
$theEmail = $_POST["theEmail"];

$aDBLink = @mysql_connect("localhost" , "root") ;

if ( !empty( $aDBLink) ) {
if ( mysql_select_db( "database", $aDBLink ) == True ) {
$aSQL = "SELECT * FROM member WHERE $theUsername = username" ;
$aQResult = @mysql_query($aSQL, $aDBLink ) ;

if (mysql_num_rows($aQResult) == 0) {
$aSQL = "INSERT INTO member VALUES ( null, '$theFirstName' , '$theLastName' , '$theUsername', '$thePasswordm', '$theEmail')" ;
$aQResult = @mysql_query($aSQL, $aDBLink ) ;
$member_id = mysql_insert_id ($aDBLink) ;

} else {
echo '<p>the username is already taken</p>' ;
}
}

When I input some data in the register form and go 'Register', it's not working and it comes up with "Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in c:\program files\apache group\apache\htdocs\memberregister.php on line 34"

Can anyone tell me what's wrong with the code? Thanks