Page 1 of 1

Does my script look safe???

Posted: Thu Oct 20, 2005 1:58 pm
by Ralle
Hello!
I would like you to tell me if there are any security holes in this little script!

Code: Select all

<?

//   CREATE TABLE `tekst` (
//  `text_id` int(100) NOT NULL auto_increment,
//  `text_string` varchar(100) NOT NULL default '',
//  `text_approve` int(100) NOT NULL default '0',
//   PRIMARY KEY  (`text_id`)
// ) TYPE=MyISAM AUTO_INCREMENT=23 ;


echo '<div align=center>';

//CONNECT TO DATABASE
$dbhost = "localhost";
$dbuser = "root";
$dbpass = "";
$dbname = "rulletekst";
mysql_connect("$dbhost", "$dbuser", "$dbpass") or
   die("COULD NOT CONNECT TO SERVER: " . mysql_error());
mysql_select_db("$dbname");

$string = $_POST[string];

$actionb = mysql_query("SELECT * FROM tekst");
while ($row = mysql_fetch_array($actionb, MYSQL_BOTH))
	{
	$d++;
	$all_row['text_id'][$d] = $row['text_id'];
	$all_row['text_string'][$d] = $row['text_string'];
	$all_row['text_approve'][$d] = $row['text_approve'];
	if ($string == $row['text_string']) {$error = true;}
	}
if($error == true) {echo "Error! This string does already exist!<br>";}

//POST STRING

	echo '<form action="rollingtext.php" method="post" enctype="multipart/form-data">
		<input name="string" type="text" value="">
		<input name="post" type="submit" value="Shout..!?">
		</form>';

//ERROR
if ($string != NULL && $error != true)
	{
	$actiona = "INSERT INTO tekst (text_string) VALUES ('$string')";
	mysql_query($actiona) or die('Error<br>');
	echo 'Inserting ' . $string . ' into database...<br>';
	}

//SHOW TEXT
$number = rand(1, $d);

echo '<marquee align="middle" behavior="alternate" width="200" height="33">' . $all_row['text_string'][$number] . '</marquee><br>';
echo 'The database contains: ' . $d . ' strings of random babbling<br>';
echo '</div>';
?>

Posted: Thu Oct 20, 2005 3:02 pm
by shiznatix
yo do no error checking on $string before you insert it into the database. very insecure since there is a large posibility of sql injection. use some regex to strip out anything non alphanumeric unless you want some stuff to stay. strip_tags would be a good idea along with mysql_real_escape_string. regex would be the most important part to make sure that it was going to be safe to run the query.

also you have

Code: Select all

$string = $_POST[string];//put quotes around the string in the post array
but thats just becuase it will give you a warning is error reporting is set to E_ALL - not a security threat just good practice

Posted: Thu Oct 20, 2005 7:56 pm
by Ralle
could you make some expamples on the thing you want me to do?

Posted: Fri Oct 21, 2005 9:45 am
by foobar
Ralle wrote:could you make some expamples on the thing you want me to do?
You could do something like the following before inserting into the database:

Code: Select all

$string = mysql_escape_string($_POST['string']);

Posted: Fri Oct 21, 2005 11:24 am
by Jenk
Don't forget to check for magic_quotes first!

Code: Select all

<?php

function sqlClean ($string) 
{
    if (get_magic_quotes_gpc()) {
        $string = stripslashes($string);
    }
    return mysql_real_escape_string($string);
}

?>