0

I’m trying to create a script for a user to enter in their username, and then have other logged in usernames randomly show, in a chatroulette fashion.

So, you will enter in your name and hit submit, then your name will be stored in a database and someone else’s name will be pulled out at random and shown. Then the user can hit a next button to see another random user name. When the user closes the page, their name will be unloaded from the system.

What I have tried is creating a simple post submission form which will return you to the same page logged in with your name, and it inserts your name into a mysql database. That worked.

Then I added some PHP code to detect that the name variable has been set and to find a random username in the database by finding the amount of users in the database and using a random integer to pick one out. I’m pretty sure it worked, however I was unable to get the user name to show with echo "$name";.

Then I tried adding an automatic logout by using:

<body onUnload=<?php session_destroy();?>>

That didn’t work. I didn’t get around to creating a next button because I was having a few problems, because I figured out that the logout wouldn’t work because I would be dropping rows from the database that wouldn’t be filled in again as new rows were added to the SQL database with an auto increment function causing blank pages to be shown.

Here is my code:

<html>
<head>
<title>random name</title> 
</head>
<body>
<center>
<h1>random name</h1>
<h5>By DingleNutZ</h5>
</center>
<?php
if (!isset($_POST['name'])){
echo "<form action=\"index.php\" method=\"POST\" name=\"form\"><center><h4>name:</h4><input name=\"name\" id=\"name\" type=\"text\"/><br/>
<input type=\"submit\" name=\"submit\" value=\"Play!\"/></center></form>";
}else{
$name = $_POST['name'];
$host="localhost"; // Host name 
$username="root"; // Mysql username 
$password=""; // Mysql password 
$db_name="ftr"; // Database name 
$tbl_name="players"; // Table name
// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect"); 
mysql_select_db("$db_name")or die("cannot select DB");

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

$sql="SELECT * FROM $tbl_name WHERE name='$name'";
$result=mysql_query($sql);

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


if($count==1){

session_register("name");
session_start();
if(session_is_registered(name)){
$players=mysql_query("SELECT MAX (id) FROM $tbl_name");
$chooserand=rand(1,$players);
$callee=mysql_query("SELECT name FROM $tbl_name WHERE id=$chooserand");
echo "$callee";
echo "<a href=\"index.php?playing=1\">Logout</a>";
if (isset($playing)){
if ($playing == 1){
$drop_name=mysql_query("DELETE FROM $tbl_name WHERE name=$name");
}}
}
}
echo "show random name here";
}
?>
</body>
</html>

There is a variable in there called $playing which was an attempt at a logout system.

I would be very grateful for any answers. Many thanks in advance.

as i didnt make it obvious (sorry guys) i need to fix my main problem which is being able to show a random user without ever showing a blank page due to the rows being dropped from the database. it is essential that usernames are removed from the system for privacy

DingleNutZ
  • 163
  • 1
  • 7
  • 3
    1) `"$var"` is pointless, just use `$var` (without quotes). 2) `onUnload=` doesn't work because all PHP is interpreted on the server first, then sent to the browser. You can't trigger a PHP function in an `onunload` event, because it will already have finished executing before the page reaches the browser. – deceze Nov 03 '11 at 08:17
  • 1
    I think what you should do is examine the PHP session features, and not try to determine whether the window is open or closed yourself. – miahelf Nov 03 '11 at 08:18
  • You could also do something like this, onUnload="myJavascriptFunction()" - and the Javascript function would make an AJAX call to a PHP file, which would then run some PHP code for you. – miahelf Nov 03 '11 at 08:20
  • “I would be very grateful for any answers.” I don’t know anything about PHP, but I can’t tell what your actual question is. – Paul D. Waite Nov 03 '11 at 08:36
  • This: `$drop_name=mysql_query("DELETE FROM $tbl_name WHERE name=$name");` Is A: a syntax error B: an SQL-injection hole. If you don't quote your `'$vars'` in a query your code will fail for non-integer values and `mysql_real_escape_string()` cannot protect you. – Johan Nov 03 '11 at 09:54
  • Did seeing `>` brighten up anyone else's morning? – Ross Nov 03 '11 at 11:23

1 Answers1

2

You have a few issues in your code, not all are errors as such, some code is unneeded, other code is potentially dangerous.

$name = stripslashes($name);              <<-- delete this line.
$name = mysql_real_escape_string($name);  <<-- this is all you need.

mysql_real_escape_string() is all you need. No other escaping is need to protect against SQL-injection.
A few caveats apply, which I will discuss below.

$sql="SELECT * FROM $tbl_name WHERE name='$name'";  
$result=mysql_query($sql);  

Select * is an anti-pattern, never use it in production code. Explicitly select the fields you need.
You are using dynamic tablenames, I fail to see the need for this and it's also a dangerous SQL-injection hole.
Never use it but if you must, see this question how to secure your code: How to prevent SQL injection with dynamic tablenames?
You do the query, but you don't test if it succeeds, put a test in there:

$sql = "SELECT id FROM users WHERE name='$name' ";  
$result = mysql_query($sql);  
if ($result) 
{
  $row = mysql_fetch_array($result);
  $user_id = $row['id'];  
}
else { do stuff to handle failure }

You are trying to get data out of the database, but this is not the way to do it:

$players = mysql_query("SELECT MAX (id) FROM $tbl_name");                
$chooserand = rand(1,$players);                
$callee = mysql_query("SELECT name FROM $tbl_name WHERE id=$chooserand");                
echo "$callee";         

But I see a few issues:
Please stop using dyname tablenames, it is a really bad idea.
The return value of mysql_query is a query_handle, not the actual data you're quering.
I would suggest escaping all values, whether from outside or inside your code; I know this is paranoid, but that way, if you code design changes, you cannot forget to put the escaping in.
Never ever ever echo unsanitized data in an echo statement.
If you echo a $var, always sanitize it using htmlentities. If you don't XSS security holes will be your fate.
See: What are the best practices for avoiding xss attacks in a PHP site

rewrite the code to:

$result = mysql_query("SELECT MAX (id) as player_id FROM users"); 
$row = mysql_fetch_array($result);
$max_player = $row['player_id'];               
$chooserand = mysql_real_escape_string(rand(1,$max_player)); 
//not needed here, but if you change the code, the escaping will already be there.
//this also makes code review trivial for people who are not hep to SQL-injection.               
$result = mysql_query("SELECT name FROM users WHERE id = '$chooserand' ");
$row = mysql_fetch_array($result);  
$callee = $row['name'];            
echo "callee is ".htmlentities($callee);  

Finally you are deleting rows from a table, this looks like a very strange thing to do, but it is possible, however your code does not work:

$drop_name = mysql_query("DELETE FROM $tbl_name WHERE name=$name");  

As discussed mysql_query does not return values.
On top of that only a SELECT query returns a resultset, a DELETE just returns success or failure.
All $vars must be quoted, this is a syntax error at best and an SQL-injection hole at worst.
Technically integers don't have to be, but I insist on quoting and escaping them anyway, because it makes your code consistent and thus much easier to check for correctness and it elimiates the chance of making errors when changing code
Rewrite the code to:

$drop_name = $name;
$result = mysql_query("DELETE FROM users WHERE id  = '$user_id' ");  
//user_id (see above) is unique, username might not be.
//better to use unique id's when deleting.
$deleted_row_count = mysql_affected_rows($result);  
if ($deleted_row_count == 0)
{ 
  echo "no user deleted"; 
} else {
  echo "user: ".htmlentities($drop_name)." has been deleted";
}
Community
  • 1
  • 1
Johan
  • 74,508
  • 24
  • 191
  • 319
  • thankyou very much, i will accept your answer if you can help me with my main problem which is being able to show a random user without ever showing a blank page due to the rows being dropped from the database. it is essential that usernames are removed from the system – DingleNutZ Nov 03 '11 at 11:20
  • I disagree with the stripslashes; if magic_quotes_gpc is enabled, stripslashes would remove the erroneously inserted backslashes before inserting into the database. Yes, turning off magic_quotes_gpc is the preferred solution, but stripslashes is a good second contender. – Berry Langerak Nov 03 '11 at 11:29
  • @BerryLangerak, I see your point, but the issue is that magic_quotes should never be enabled. Using stripslashes just hides the fact that it is enabled. Better to remove it and debug the issue. – Johan Nov 03 '11 at 11:49
  • 1
    @DingleNutZ, All the info you need is in the answer, I really don't care **that** much if you accept the answer or not. I think I've put in enough effort. – Johan Nov 03 '11 at 11:50
  • @Johan True, magic_quotes should simply be disabled. Nonetheless, this need not be a decision he can make? If not, stripslashes remains a good solution. But, if he can, disabling magic_quotes is always the way to go :) – Berry Langerak Nov 03 '11 at 12:19
  • you have put in a lot of effort, your a legend. i will be making sure i credit your help and advice – DingleNutZ Nov 03 '11 at 12:35