Checking for duplicates

Other Tech Topics
1

Hey all,
I need some help with a SQL issue, A close friend got a pre-wriiten website which has a issue, he asked me to look and I think I have found a fault and its in the SQL code, had a closer look to see if it was a spelling mistake or something easy to spot but can't see anything and SQL was never my strong point, I don't think its checking correctly, or its checking creating a new username then chanking again and if its duplicate not doing a 3rd check just issuing the username,

The basic issue it when someone clicks a link on the webpage its supposed to generate a username for them, now the first part of the username is fixed to the site name (not with www. or .com etc) and give a random string of numbers between 0-9000 so it would look like "user-123456" or "user-321" and then check to see if that username has already been given, if so generate a new username and check again and keep doing that untill it gives a unique username.

The problem is its not doing that, its generating the username OK but its not checking for duplicates and some duplicate usernames have started to appear causing problems. Each username is a different password (thats randomly generated too) although no duplicate passwords have been generated.

Here is the code which is in the php file:

$chkpay = $row[0];
$resu2lt = mysql_query("SELECT * FROM skylines WHERE ownerid=$myid AND paidline='5'");
$num_rows = mysql_num_rows($resu2lt);
if ($num_rows >= 2) {
die("Error you have reached your limit of UNPAID new lines Click here to go back");

			} else {

			// generate name
			$linename = $uname ."-". rand(0,9000);
			// check if name is in use
			$resu2lt = mysql_query("SELECT * FROM skylines WHERE ownerid=$myid AND lineuser='$linename'");
			$num_rows = mysql_num_rows($resu2lt);
			if ($num_rows == 1) {				
			$linename = $uname ."-". rand(0,90000);
			// check if name is in use again
			$resu2lt = mysql_query("SELECT * FROM skylines WHERE ownerid=$myid AND lineuser='$linename'");
			$num_rows = mysql_num_rows($resu2lt);
			if ($num_rows == 1) {				
			$linename = $uname ."-". rand(0,900000);
			} }

			$linepass = rand(100,9000000);
			$linestart = date("d-m-Y");
			$lineexpire = mktime(date("H"), date("i"), date("s"), date("m"), date("d"), date("Y"));

			
			mysql_query("INSERT INTO skylines (ownerid, lineuser, linepass, startdate, expiredate, linestatus, linetest, paidline) 
			VALUES 
			('$myid', '$linename', '$linepass', '$linestart', '$lineexpire', 'New-Line', '3', '5')");

Hope my description of the what it should be doing what it is doing makes sense so if you can help me please if you can tell me what the error is and how to fix it that would be fantasic. I can't change the datebase names so needs a work a round please.

Thank you

2

since you are not using an explicit transaction, there is a non-zero chance that an identical username (even with the randomization) will be generated between the selects and the insert. My advice would be to write a stored procedure to do this work instead of doing it PHP. The procedure can set up a proper transaction and do the insert. It will also perform much better on a busy website and give you better, more focused security options.

3

Hi,

Thank you for taking the time to reply, SQL is my weakest point so not too sure what your getting at but as the php page is fixed into my friends website it has to stay, the is more HTML code etc other than what I posted so can't ditch it.

Is there anyway just to fix whats there? either make some changes or add a little extra to stop the duplicates?

4

Sure:

  1. Write a stored procedure to add new user ids. Put the logic there to make them unique.
  2. Change the PHP to remove the selects and insert and call the stored procedure instead.
6

Hi all, thank you for the replies. I have re-written the code to. Can someone take a look and tell me if I made any errors and it it will work.

$chkpay = $row[0];
$resu2lt = mysql_query("SELECT * FROM skylines WHERE ownerid=$myid AND paidline='5'");
$num_rows = mysql_num_rows($resu2lt);
if ($num_rows >= 2) {
die("Error you have reached your limit of UNPAID new lines <a href********>Click here to go back");

            } else {

                // wipe any non-alphanumeric characters from uname
            preg_replace("/[^0-9a-zA-Z]/","",$uname)
                            // generate name
            $num_rows = 1;
            while ($num_rows > 0)
                {
                $linename = $uname ."-". rand(0,900000);
                // check if name is in use
                $resu2lt = mysql_query("SELECT * FROM skylines WHERE ownerid=$myid AND lineuser='$linename'");
                $num_rows = mysql_num_rows($resu2lt);
                };}


            $linepass = rand(100,9000000);
            $linestart = date("d-m-Y");
            $lineexpire = mktime(date("H"), date("i"), date("s"), date("m"), date("d"), date("Y"));

            
            mysql_query("INSERT INTO skylines (ownerid, lineuser, linepass, startdate, expiredate, linestatus, linetest, paidline) 
            VALUES 
            ('$myid', '$linename', '$linepass', '$linestart', '$lineexpire', 'New-Line', '3', '5')");
            // echo "Your new line has been created <a href=*****>Click here</a> to view your new line";

?>

7

It suffers from the same problem as before: It is possible on a busy system for the same userid to be generated by two different processes. You need a transaction-based approach and you should move the SQL logic into a stored procedure that generates the userid, inserts it into skylines and returns the generated userid to the caller.