You have an error in your SQL syntax - help!!

[runnerjp]

Hey guys.

 

My site got hacked at the weekend, thankfully all they did was delete my test posts and post a message on the forum. Been trying to find how they did it with my login script ect but seems to me they have not cracked the login screen as it seems tight.

 

Then i think i found out how.  i use page= withing my script so thought i would try the adding of ' at the end of the script. This then comes up with the error message:

 

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'name',`user_id`='number'' at line 1.

 

Im right in saying they can change anything in my sql database with this loop hole cant they!

 

 

What im asking is how can i fix this??

 

 

Regards

 

Jarratt

[Mchl]

Oh just delete this comma here.

 

Where?

 

Oh... You didn't post your code, so I'm not posting mine

[RussellReal]

mysql_real_escape_string but also if you're expecting an integer you might aswell typecast the variable $id = (int) $_GET['id'];

[JonnoTheDev]

What you have experienced is called SQL injection. They have used part of a SQL query as a url parameter. If your site was live they could have dropped the whole database.

ALWAYS, ALWAYS, ALWAYS Escape Input & Filter Output! In other words never trust any data that comes from a url parameter, form input, cookie, etc.

Look at php's functions to escape & filter data. i.e

 

// strip any html tags
$x = strip_tags($x);
// escape for database insert
$x = mysql_real_escape_string($x);
// convert any special tags to their html entity
$x = htmlentities($x);
// look for certain characters and remove them
$x = str_replace("bad word", "", $x);
// check if the value is a number
if(is_numeric($x)) { }
// check if the value contains only letters
if(ctype_alpha($x)) { }

Search Google for tutorials on making data safe for the likes of database queries.

[PFMaBiSmAd]

The fact that a ' will break the SQL syntax means that you are not escaping string data being put into a query. That also means that SQL can be injecting into your queries. The SQL that is injected can be used to dump all the contents of your table by adding a UNION to a SELECT query and it can also be used to cause a SELECT query to return ANY matching row instead of an exact matching row, such as in a login script.

 

So, for the query that is failing, are you using mysql_real_escape_string() on each piece of string data being put into it that could have come from the visitor via GET, POST, COOKIE, FILES, SERVER, ENV variables or indirectly through SESSION variables that were at one time GET, POST, ... variables?

 

Edit: Because mysql_query() does not support multiple queries separated by ; it is not directly possible to inject sql that will do things like drop tables.

[runnerjp]

Right so all data input i need to mysql_real_escape_string and that should sort the problem. Would there be any way of avoiding the You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'name',`user_id`='number'' at line 1.

 

Coming up or would mysql_real_escape_string stop this from coming up anyway?

 

@Mchl - sorry for supplying no code , but didnt know if it would help or just bulk up the post

 

[PFMaBiSmAd]

Using mysql_real_escape_string() on string data will only prevent the sql syntax from breaking and will prevent sql injection via string data. SQL can still be injected in numeric data (see RussellReal's post above.)

 

Someone can still trigger mysql errors by doing things like injecting a large amount of data that exceeds the max_allowed_packet or can cause 'server has gone away' errors.

 

In general, you should not cause the output from mysql_error() to appear on a live web site (display_errors should be OFF and log_errors should be ON and your logic should cause mysql_error() to use a php function that makes use of the error_reporting/display_errors/log_errors settings) and you should use error checking logic on all your queries (and in fact on all functions that can fail) to detect if an error occurred, output a meaningful user error message when it does, report/log system level information so you can find and fix problems, and take an appropriate action in the remainder of your code on the page.

 

Typical logic to do this for SELECT, SHOW, DESCRIBE, EXPLAIN... type queries -

 

<?php
$query = " ... "; // your query in a string variable (makes error reporting easier because you can log or display the actual query)
// execute the query and check for success or failure
if($result = mysql_query($query)){
// the query executed without any errors and returned a result resource (SELECT, SHOW, DESCRIBE, EXPLAIN... type queries)
// check if any rows were returned by the query (SELECT... type queries)
if(mysql_num_rows($result)){
	// the result set contains one or more rows
	// process the row(s) in the result set here...
} else {
	// no rows were matched, output a helpful user error message
	echo "The query did not match any rows in the database<br />";
}
} else {
// the query failed and returned a FALSE value
// do your application level error reporting here...
trigger_error("Query failed: $query<br />" . mysql_error());
// output a user error message
echo "The query could not be executed at this time due to an error...<br />";
}
// the remainder of the code on the page having nothing to do with the above query logic
?>

 

Typical logic to do this for INSERT, UPDATE, DELETE, DROP... type queries -

 

<?php
$query = " ... "; // your query in a string variable (makes error reporting easier because you can log or display the actual query)
// execute the query and check for success or failure
if($result = mysql_query($query)){
// the query executed without any errors and returned a TRUE value (INSERT, UPDATE, DELETE, DROP... type queries)
// check if any rows were affected by the query (INSERT... type queries)
if(mysql_affected_rows()){
	// one or more rows were affected, do success processing...
	// output a success user message -
	echo "The row was successfully UPDATED in the table<br />";
} else {
	// no rows were affected, do failure processing...
	// output a user error message
	echo "The query could not UPDATE the row in the table (likely because the WHERE clause is FALSE)<br />";
}
} else {
// the query failed and returned a FALSE value
// do your application level error reporting here...
trigger_error("Query failed: $query<br />" . mysql_error());
// output a user error message
echo "The query could not be executed at this time due to an error...<br />";
}
// the remainder of the code on the page having nothing to do with the above query logic
?>

[Mchl]

@Mchl - sorry for supplying no code , but didnt know if it would help or just bulk up the post

 

Not a problem. Seems like other guys are kind enough to help you without seeing it

[runnerjp]

Sorry about this but on this page can any 1 tell me what i need to chnage to prevent the erro message 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'Admin',`user_id`='1'' at line 1' appearing?  as i cant seem to find the area involved/

 

cut down code as much as possible

<link rel="stylesheet" type="text/css" href="http://www.runningprofiles.com/css/login.css"> 
<link rel="stylesheet" type="text/css" href="http://www.runningprofiles.com/members/include/style.css"> 


<?php
//look to see if the forum is currently locked
$sQry = "SELECT `locked` FROM forum_lock LIMIT 1";
$obQry = mysql_query($sQry) or die(sprintf("Could not query forums (%d): %s",mysql_errno(),mysql_error()));
$record = mysql_fetch_array($obQry);


if (isset($record['locked']) && $record['locked'])
	{
	die("Sorry, the forums are currently locked."); //error message
	}
else
{


//Here we count the number of results
$data = mysql_query("Select * from forumtutorial_posts where parentid='0' AND forum = '$forum' ORDER BY important, lastrepliedto")or die("Could not get users");
$rows = mysql_num_rows($data);
$page_rows = 25; //This is the number of results displayed per page
$pagenum = $_GET['pagenum']; //This sets the range to display in our query

	if ($pagenum === "last")
	{
		$query = "Select COUNT(*) as C from forumtutorial_posts where parentid='$id'";
		$result = mysql_query($query);
		$data = mysql_fetch_array($result);
		$pagenum = ceil($data['C'] / $page_rows);
			}
			$pagenum = (is_numeric($pagenum) && $pagenum >= 1) ? (int)$pagenum : 1;
			$max = 'limit ' . ($pagenum - 1) * $page_rows . ',' . $page_rows;




			{
/* gets users online */
			$getusersonline = "SELECT user_id,user FROM useronline    WHERE  file = 'http://www.runningprofiles.com/members/index.php?page=forum&forum=$forum' AND
			timestamp > " . (time() - 900); //grab from sql users on in last 15 minutes
			$getusersonline2 = mysql_query($getusersonline) or die("Could not get users");
			$num = mysql_num_rows($getusersonline2);


$getthreads = "Select * from forumtutorial_posts where parentid='0' and forum = '$forum' ORDER BY important ASC, lastrepliedto DESC $max";

$getthreads2 = mysql_query($getthreads) or die("Could not get threads");
while ($getthreads3 = mysql_fetch_array($getthreads2))
{

										$important = $getthreads3['important'];

											$query1 = mysql_query("SELECT COUNT(postid) FROM forumtutorial_posts WHERE( postid= '$getthreads3[postid]' OR parentid = '$getthreads3[postid]' ) AND author='$username'");
$count = mysql_result($query1, 0, 0);
echo ($count != 0)  ? '<img src="/images/posted.jpg" />' : '<img src="/images/posted2.jpg" />'; 
  ?>

[Mchl]

Are you sure it's on this page? No query here looks like the one from error message.

[runnerjp]

Yes i enter  http://www.runningprofiles.com/members/index.php?page=forum&forum=General'  ( note the ' )  and it comes up with this error 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'Admin',`user_id`='1'' at line 1'

[Mchl]

This error message comes from mysql_error function. Only query in your code that this function is used after is:

SELECT `locked` FROM forum_lock LIMIT 1

, which is neither wrong, nor affected by any user input variables.

 

Perhaps some other php file is being included by your script? Try looking for other usages of mysql_error

[runnerjp]

Wierd now when i enter '  i dont get the error message :S

 

 

[Mchl]

Please see 'PHP Debugging: A Beginner's guide' from my signature for some tips on how to find out where errors occur.

[runnerjp]

Sorry i should point out im tryin to cause the error as my website was hacked

[Mchl]

Yeah, but right now you don't even know where errors happen when you provoke them. That's pretty essential thing to know.

[PFMaBiSmAd]

If you go back to my last post in this thread and expand upon the // do your application level error reporting here... so that it includes the 5 W's - Who (the logged in username, IP address..), What (the actual query...), When (date/time...), Where (file name and line number), and Why (the result or error...) about the function call that is failing, you will know who caused the error, what data caused error, when it occurred, where it occurred, and why it occurred.

[PFMaBiSmAd]

Umm. Don't loose sight of the fact that the error message is just the end result of your code not escaping data being put into a query. The data you are testing with by adding a ' on the end of the URL happens to cause a syntax error. A hacker will be injecting actual SQL, which won't trigger an error, but will in fact either dump data in your tables or cause any username/password combination to cause him to become logged in as you.