tinyang wrote:You are absolutely correct! I was hoping to get it working before adding security to it, I'm not sure I understand the proper use of magic quotes and MRES yet.
It's worth just putting it in from the start, it can help avoid mysql errors such as this one.
Basically, if the variable you're expecting is a number, make sure its a number or cast it to one. By ensuring it's a number you know its safe to put in your query without quotes, e.g.:
Code: Select all
$num = $_POST['varname'];
if(!is_numeric($num)) {
// its not a number - either set to a default number, or exit with an error
$num = 0;
}
// alternatively, cast to a float or integer:
$num = (int)$_POST['varname']; // will return 0 if $_POST['varname'] isn't a number
$num = (float)$_POST['varname']; // will also return 0 if $_POST['varname'] isn't a number
It's a good idea to apply mysql_real_escape_string to all variables going into a query, regardless of whether its a number, date, binary or anything else! that way any quotes and other special characters will be escaped so MySQL knows its part of a query value.
The following example shows why this is so important. It's intended to get the row from the mysql table but only where the username matches $var.
Code: Select all
$var = $_POST['username'];
$qry = "SELECT * FROM mytable WHERE username='$var';";
mysql_query($qry);
But, what if somebody submits
as the value of $_POST['username']? Well, the query becomes this:
Code: Select all
SELECT * FROM mytable WHERE username = '' OR 1=1;#';
which will return all rows, as in every case 1=1 will be true. Everything after the # is ignored.
If you'd run mysql_real_escape_string on $var before passing it to the query, the quotes will have been escaped and you're query would function as intended.
tinyang wrote:Another good point, where would I find the logs? My server is fedora running apache.
hard to say. my fedora server was putting them into /var/logs/ iirc, though my ubuntu one puts them somewhere different. is it a server you've setup yourself or a dedicated/shared server? if the former, have a look in your apache config files for the path. if the latter, you may be able to find out using phpinfo() or your hosts help files.
And finally, this line doesn't look quite right:
Code: Select all
$query="INSERT INTO comments (pic_id, user, rating) VALUES ('.$pic_id.', '".$_SESSION['username']."', '.$rating.';)";
I don't think there should be a semi-colon before the closing bracket at the end. It should be after the bracket not before, e.g.
Code: Select all
$query="INSERT INTO comments (pic_id, user, rating) VALUES ('.$pic_id.', '".$_SESSION['username']."', '.$rating.');";
Other than that, unless one of the values of either $pic_id, $rating or $_SESSION['username'] are incorrect, I can't see anything obviously wrong with your code.
Instead of using die(mysql_error()) it may be worth outputting it manually, e.g.
Code: Select all
$qry = 'select whatever blah blah';
$res = mysql_query($qry);
if(mysql_error()) {
echo "MySQL error: ".mysql_error();
}
(do the same with your mysql_connect and mysql_select_db lines too!). that should force the error to go to output rather than the error logs
hth