What does the script output (the relevant part (the anchor with onclick) of the resulting HTML)?
Also,
PHP Code:
$div_id = isset( $_GET['id'] )? $_GET['id']: false;
$rating = isset( $_GET['rating'] )? $_GET['rating']: false;
$stars = isset( $_GET['stars'] )? $_GET['stars']: false;
$report = isset( $_GET['report'] )? $_GET['report']: false;
are *all* of these variables passed in from the query string? From your original post, it seemed only $rating
and $div_id
came from the query string. If $stars
and $report
are not supposed to (and from your code, I doubt that they are), then allowing them to can cause problems later on.*
My recommendation above was to simply set an empty string for $stars
, and to change the flow of your script to avoid trying to print $report
if you hadn't defined it (possibly by giving it a default value to start with).
*a quick explanation about the possible dangers of what you're doing...
I should have mentioned this in the first place, since your current code is *very* vulnerable to both SQL injection and cross-site scripting (XSS) attacks.
Take a closer look at how you're building your SQL queries:
PHP Code:
"SELECT id FROM ratings WHERE div_id = '" . $div_id . "' AND ..."
Here, $div_id
is simply the value the user sent you in $_GET['id']
. You're probably expecting a number, but you don't confirm (validate) that, nor do you escape (sanitize) the data before sending it to your database.
Rule #1 is never trust user input. Sometimes it's malicious. Many, many times, it simply contains mistakes.
What if I visit www.yoursite.com/this_script.php?id=what'sup? your finished query becomes:
Code:
SELECT id FROM ratings WHERE div_id = 'what'sup' AND...
there's an extra single quote in there now, throwing off where your strings start and stop. This will cause an error in your SQL.
Now imagine what would happen if I'd visited www.yoursite.com/this_script.php?id=' OR (DELETE FROM ratings WHERE 1=1)#.
You need to escape strings you send to your database when they might contain dangerous characters.
For ext_mysql, use
mysql_real_escape_string( $str )
.
However, ext_mysql is outdated and scheduled to be deprecated. As soon as is practical, you should switch to an up-to-date mysql library like mysql
i or PDO. Read more about
choosing an API for MySQL.
Similar problems exist when you're taking user-supplied input and returning it to the HTML page without validating and sanitizing it, as happens when you print these lines:
Code:
<a onClick=\"javascript:addRating('$div_id','$x');\" class=\"star$x\">$x</a>
what if I visit www.yoursite.com/this_script.php?id=','5'); (function( a,b ){ steal_login_cookies(); })('0 [COLOR="#808080"]?
Always validate that what you get from the user is safe to display.
Make use of functions like
strip_tags() and
htmlspecialchars() to make sure output does not include markup/script.
The examples I gave above don't exactly work, but that's intentional. YES, it can be done. These very security issues have cost companies millions of dollars.
Bookmarks