Jump to content


Photo

Possible SQL injection error?

sql injection mysql server version sql syntax

Best Answer Fermac, 27 August 2013 - 09:18 AM

2 different sections of code needed fixed for the cart to work correctly.

 

Firstly, the displaying of the cart was not selecting any of the items from the database that associated with the cart ID. It was re-written to select only the details needed to keep code to a minimum and also to be a lot cleaner with correct error debugging.

 

Secondly, the updating of the cart when the "Update" button was pressed. It was taking the long way around a short simple task, it was performing updates on items that hadn't changed quantities, it was re-written as follows:

- Quantities are changed to between 0-10 and update is submitted.

- Script checks to see which quantities have changed (if any).

- If there are changes to the quantities, the script selects which items have been changed.

- It then loops through all of the quantity changes and either removes the row (if the value is 0) or updates the row (i the values is 1 or more).

Go to the full post


  • Please log in to reply
29 replies to this topic

#1 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 08:43 AM

Hi all,

   I am new to this, so please be gentle :) . I have just taken over a website and moved to new server and it has thrown up a couple of errors, most of which I've sorted other than this one:

 

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 '' at line 1

 

this comes up when trying to update a quantity in a shopping cart and I think it has something to do with SQL injection but I've no idea what to do. My two bits of code are below, first the page with the drop down menu on it:

<?php include 'include/functions.php';
$colname_rsOrderInformation = "-1";
if (isset($_SESSION['cartId'])) {
  $colname_rsOrderInformation = $_SESSION['cartId'];
}
mysql_select_db($database_conDB, $conDB);
$query_rsCartInformation = sprintf("SELECT * FROM tabCart WHERE cartId = %s", GetSQLValueString($colname_rsOrderInformation, "int"));
$rsCartInformation = mysql_query($query_rsCartInformation, $conDB) or die(mysql_error());
$row_rsCartInformation = mysql_fetch_assoc($rsCartInformation);
$totalRows_rsCartInformation = mysql_num_rows($rsCartInformation);
?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta name="keywords" content="belfast, cookery, school, fish, seafood, learning, northern ireland" />
<meta name="description" content="Belfast Cookery School in association with Mourne Seafood Bar - More than just a restaurant" />
<title>Belfast Cookery School in association with Mourne Seafood Bar - More than just a restaurant - Gift Vouchers</title>
<link rel="shortcut icon" href="favicon.ico" />
<link href="main.css" rel="stylesheet" type="text/css"/>

<style type="text/css">
.slideshow { height:319px; width:476px; margin-right:0px; margin-bottom:0px;  z-index:0; background-color: #fff;}
.slideshow img { height: 319px !important; width: 476px !important; padding-left: 0px;  }
</style>
<!-- include jQuery library -->
<script type="text/javascript" src="include/jquery.min.js"></script>
<!-- include Cycle plugin -->
<script type="text/javascript" src="include/java.js"></script>
<script type="text/javascript">
$(document).ready(function() {
    $('.slideshow').cycle({
		fx: 'fade' // choose your transition type, ex: fade, scrollUp, shuffle, etc...
	});
});
</script><?php if (!isset($_SESSION['memUsername'])) {?>
<script type="text/javascript">
function changeToPassword(){
document.getElementById("txtLogPassword").type = "password";
document.getElementById("txtLogPassword").value = ""
}

function resetPassword() {
	if (document.getElementById("txtLogPassword").value == "") {
		document.getElementById("txtLogPassword").type = "text";
		document.getElementById("txtLogPassword").value = "Password";
	}
}
</script>
<?php }?>
<script type="text/javascript">


  var _gaq = _gaq || [];
  _gaq.push(['_setAccount', 'UA-9218136-12']);
  _gaq.push(['_trackPageview']);


  (function() {
    var ga = document.createElement('script'); ga.type = 'text/javascript'; ga.async = true;
    ga.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.google-analytics.com/ga.js';
    var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(ga, s);
  })();


</script>

Now the update function that's called:

<?php 
for ($x=1; $x<=$_POST['hidTotalCartItems']; $x++) {
	$tempCartQty = "selQty".$x;
	$newCartQty = @$_POST[$tempCartQty];
	
	$tempCartItemId = "hidCartRowId".$x;
	$cartItemId = @$_POST[$tempCartItemId];
	
	$tempCartItemPrice = 'hidCartPrice'.$x;
	$cartItemPrice = @$_POST[$tempCartItemPrice];
	
mysql_select_db($database_conDB, $conDB);
$query_rsUpdateCartLine = sprintf("SELECT * FROM tabCart WHERE Id = %s", $cartItemId);
$rsUpdateCartLine = mysql_query($query_rsUpdateCartLine, $conDB) or die(mysql_error());
$row_rsUpdateCartLine = mysql_fetch_assoc($rsUpdateCartLine);
$totalRows_rsUpdateCartLine = mysql_num_rows($rsUpdateCartLine);

if ($newCartQty>0) {
	$newCartItemPrice = $newCartQty * $cartItemPrice;
	
	$updateSQL = sprintf("UPDATE tabCart SET cartQty=%s, cartTotal=%s WHERE Id = %s",
                       GetSQLValueString(@$newCartQty, "int"),
                       GetSQLValueString(@$newCartItemPrice, "double"),
					   GetSQLValueString($cartItemId, "int"));

  mysql_select_db($database_conDB, $conDB);
  $Result1 = mysql_query($updateSQL, $conDB) or die(mysql_error());
  
  mysql_free_result($rsUpdateCartLine);
}
	
	if ($newCartQty==0) {
		$deleteSQL = sprintf("DELETE FROM tabCart WHERE Id = %s",
					   GetSQLValueString($cartItemId, "int"));

  mysql_select_db($database_conDB, $conDB);
  $Result1 = mysql_query($deleteSQL, $conDB) or die(mysql_error());
	}

}

?>

Anyone got any ideas what I'm doing wrong? When the voucher quantity is changed and update button clicked, that's when the error occurs. 



#2 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 08:51 AM

If your not injecting SQL, it's not an SQL injection error, this is most likely a GIGO error - change your update error message to the following and we'll see exactly what's going on with your SQL (just remeber and post it up ;) ).

 

$Result1 = mysql_query($updateSQL, $conDB) or die($updateSQL."<br><br>Caused an error on server, that error was :<br><br>".mysql_error());

Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)

#3 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 09:02 AM

Hi Muddy, thanks for quick response, told you I didn't know what I was talking about! Only thing comes up when I do that is:

Caused an error on server, that error was :

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 '' at line 1


#4 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 09:21 AM

so that meens that $updateSQL is empty at the time when it is being sent to SQL server - which will get it upset.

 

Remove the @ suppression from all the lines with the GetSqlValString and see what happens


Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)

#5 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 09:21 AM

Not sure if this helps, but worked perfectly fine on the previous host! 



#6 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 09:24 AM

Sorry for being stupid, but I can't find GetSqlValString anywhere in my code!



#7 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 09:34 AM

    $updateSQL = sprintf("UPDATE tabCart SET cartQty=%s, cartTotal=%s WHERE Id = %s",
GetSQLValueString(@$newCartQty, "int"),
GetSQLValueString(@$newCartItemPrice, "double"),
                     GetSQLValueString($cartItemId, "int"));

 

Sorry, it is GetSQLValueString.


Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)

#8 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 09:38 AM

Removed the @ and it's still coming up with same response as last time, an empty value beside "that error was : "



#9 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 10:01 AM

not really seeing what's going on here, I can only assume errors are not being displayed on the server.  Can you show the GetSQLValueString() function decleration? and how it is being attached to your update script?


Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)

#10 mac_gyver

mac_gyver

    Staff Alumni

  • Staff Alumni
  • 3,748 posts

Posted 26 August 2013 - 10:14 AM

there's really no way that the posted code would not display something for $updateSQL in the error output. are you 100% sure the posted code is what is actually running on the server?

 

also, since the mysql_query() statements, in question, are using the $conDB connection variable, you must use that in the mysql_error() statements to insure that the error message corresponds to the last query that was executed on that connection. i'm thinking the mysql_error message you are seeing is left over from some login code or similar that is using a different connection.

 

i would also recommend that you set php's error_reporting to E_ALL and display_errors to ON to get php to help you by reporting and displaying all the errors it detects.


multi-purpose programming fool. well written source-code should be self-documenting. well written code should be self-troubleshooting.

#11 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 10:54 AM

I was using the 

die($updateSQL."<br><br>Caused an error on server, that error was :<br><br>".mysql_error());

I tried using the mysql_query and it came up with this:

 

Query was empty



#12 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 10:54 AM

Don't know if it matters but site is using PHP 5.3.27



#13 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 11:08 AM

this is not a versioning problem - this is a coding problem.

 

what mac_gyver was saying about the $conDB is that you need to put it inside the parenthesis of the mysql_error($conDB) <-- like that to make sure it is showing the relevent error from the server.

 

what you also need to do is turn on error reporting (to report on all errors) and enable display_errors in order to effictvly debug your code.  These should be turned on whenever you are developing any code (be it a new write or changes to an existing script) to make sure everything is running correctly.


Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)

#14 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 11:19 AM

Sorry I wasn't more specific, yes I changed it to 

 

mysql_error($conDB)

 

and then that's when the "Query was empty" message came up. This was a system inherited from someone else. I'm aware coding isn't great on it, but don't know enough about this to fix. I did add this to my .htaccess file as described by my host to help:

php_flag display_errors on
php_value error_reporting E_ALL

But it still came up with the same "Query was empty" message.



#15 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 11:28 AM

And only reason why I thought it was a versioning problem was because it worked fine on the last host of the person who had it, if that makes sense?



#16 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 11:37 AM

as well as going back to my post at #9, change the or die statement so it looks like this:

or die($updateSQL."<br><br>Caused an error on server, that error was :<br><br>".mysql_error($conDB)."<br><br> when running with a \$newCartQty of $newCartQty");

Edited by Muddy_Funster, 26 August 2013 - 11:37 AM.

Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)

#17 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 11:43 AM

This is what appears:

Caused an error on server, that error was :

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 '' at line 1

when running with a $newCartQty of 1


#18 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 11:57 AM

and post #9?

 

 

... Can you show the GetSQLValueString() function decleration? and how it is being attached to your update script?


Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)

#19 Fermac

Fermac

    Member

  • Members
  • PipPip
  • 19 posts

Posted 26 August 2013 - 11:59 AM

Sorry, here it is:

function GetSQLValueString($theValue, $theType, $theDefinedValue = "", $theNotDefinedValue = "") 
{
  $theValue = get_magic_quotes_gpc() ? stripslashes($theValue) : $theValue;

  $theValue = function_exists("mysql_real_escape_string") ? mysql_real_escape_string($theValue) : mysql_escape_string($theValue);

  switch ($theType) {
    case "text":
      $theValue = ($theValue != "") ? "'" . $theValue . "'" : "NULL";
      break;    
    case "long":
    case "int":
      $theValue = ($theValue != "") ? intval($theValue) : "NULL";
      break;
    case "double":
      $theValue = ($theValue != "") ? "'" . doubleval($theValue) . "'" : "NULL";
      break;
    case "date":
      $theValue = ($theValue != "") ? "'" . $theValue . "'" : "NULL";
      break;
    case "defined":
      $theValue = ($theValue != "") ? $theDefinedValue : $theNotDefinedValue;
      break;
  }
  return $theValue;
}
}


#20 Muddy_Funster

Muddy_Funster

    Advanced Member

  • Members
  • PipPipPip
  • 3,383 posts

Posted 26 August 2013 - 12:16 PM

and how is that code loaded into your update page?


Please: "This doesn't work..." is not a question.  We're not the government: we don't have anything to view what your doing on your computer.  Help us help you by asking a good question.

 

"Things needed to ask a "good" question:

  • A description of the context of your issue (Optional but can be a huge help in certain circumstances)
  • What is the actual problem (Mandatory)
  • What does the code actually do (Mandatory)
  • What you think the code should do / What you want the code to do (Mandatory)
  • What things have you tried so far (Optional, but missing it out just wastes your time and ours)
  • The actual code as you are running it - minus any personal information like Database Login Credentials (Mandatory - don't just post pseudo, the vast majority of issues are syntax and not logic)
  • As much info about your development environment as you can give - even if it's just letting us know you are using a hosting provider instead of a local install (Optional, but some questions can not be answered without it.)




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users