...

View Full Version : SQL query problem



chump2877
02-16-2006, 02:23 AM
This is driving me crazy and its probably something stupid...I have the following lines of code:


$query = 'UPDATE '. $_SESSION['username']. ' SET price='. $_POST['price']. ', description='. $_POST['description']. ' WHERE ref_no='. $index_no2;
$result = mysql_query($query);
if (mysql_errno())
{
echo $query;
die("<br>" . mysql_errno() . ": " . mysql_error() . "<br>");
}

A MySQL error occurs, so I get the following text/error message:


UPDATE username_table SET price=191.53, description=LCD 15\'\' Monitor WHERE ref_no=2
1064: You have an error in your SQL syntax near '15\'\' Monitor WHERE ref_no=2' at line 1

what is the error in the query? My guess is something is going on with the quotes inside the query, like they are not being escaped....but the slashes indicate they are escaped, right?

fci
02-16-2006, 02:32 AM
I find writing it like this is more readable(using double quotes and remember to take the single quotes out of the array keys):


$query = "UPDATE $_SESSION[username] SET price='$_POST[price]', description='$_POST[description]' WHERE ref_no='$index_no2'";

the error is you need single quotes around the string. the string may have been escaped but you didn't encapsulate the entire string with single quotes.
this is probably how you want it to look:

UPDATE username_table SET price='191.53', description='LCD 15\'\'' Monitor WHERE ref_no='2'
I tend to put single quotes around everything especially if accepting user data, what if the value is an empty string? then you'd get a query error (unless you cast it then it'd be zero, but whatever).

chump2877
02-16-2006, 03:04 AM
thanks, i knew it was something simple...:thumbsup:

Kid Charming
02-16-2006, 03:58 AM
You should never, ever drop $_POST vars directly into a db query. Always sanitize and validate your data.

fci
02-16-2006, 04:21 AM
You should never, ever drop $_POST vars directly into a db query. Always sanitize and validate your data.

I figured that's what he was doing, lazy way for anyone(not recursive):

$_POST = array_map('mysql_real_escape_string', $_POST);
although you should have a library to abstract all of this so you never have to call the ugly mysql_real_escape_string function.

chump2877
02-16-2006, 04:50 AM
These were hidden variables in the POST...not user entered...no need to validate them...

Hence,


You should never, ever drop $_POST vars directly into a db query.

That is not always true. :D

dumpfi
02-16-2006, 05:12 AM
It's easy to fake any input, including hidden variables.

For example, a user with evil intent could simply save your form site to his hard drive, change the action attribute to an absolute address to your site, and replace all inputs of type "hidden" to "text". Then he can send you modified data easily.

dumpfi

fci
02-16-2006, 05:29 AM
It's easy to fake any input, including hidden variables.

For example, a user with evil intent could simply save your form site to his hard drive, change the action attribute to an absolute address to your site, and replace all inputs of type "hidden" to "text". Then he can send you modified data easily.

dumpfi

I like the Web Developer (http://chrispederick.com/work/webdeveloper/) extension.
Right-click on page-> Web Developer-> Forms -> Display Form Details
then it's possible to edit the hidden inputs (they're converted to text). I'm sure a simple bookmarklet could achieve the same affect.

chump2877
02-16-2006, 08:01 AM
For example, a user with evil intent could simply save your form site to his hard drive, change the action attribute to an absolute address to your site, and replace all inputs of type "hidden" to "text". Then he can send you modified data easily.

that is very true....in the case of my script, if someone were to do that -- modify the hidden variables -- the application would simply fail for that person....There would be no security risk involved... the db table receiving the POST information is temporary, unique to each user, and does not contain any valuable data....The only thing someone might accomplish is to send me a whole lot of form generated spam, maybe fill up my database, and use up bandwidth....but you run that risk with any HTML form (and there are ways to prevent this)...

fci
02-16-2006, 03:32 PM
it's about best practices. i don't agree with your rationalization and a temp table for a user.. that sounds like bad design (although I don't know enough about your situation there has to be a better way).

chump2877
02-16-2006, 07:44 PM
it's about best practices. i don't agree with your rationalization and a temp table for a user.. that sounds like bad design (although I don't know enough about your situation there has to be a better way).

Then it's a bad design that works flawlessly for me....:D

Please also understand that I don;t design every app using the same methods...this app called for a different approach...normally I wouldn;t use hidden variables if the data was delicate...In fact, normally I wouldn;t use hidden variables at all because they are impractical and mostly inefficient...

Anyway, shall we agree to disagree? I wouldn;t have asked my original question if I had thought it would spark such controversy....:rolleyes:

StupidRalph
02-17-2006, 03:22 AM
although you should have a library to abstract all of this so you never have to call the ugly mysql_real_escape_string function.

Hehe I've been using the quote_smart()

// Quote variable to make safe
function quote_smart($value)
{
// Stripslashes
if (get_magic_quotes_gpc()) {
$value = stripslashes($value);
}
// Quote if not integer
if (!is_numeric($value)) {
$value = "'" . mysql_real_escape_string($value) . "'";
}
return $value;
}

How do you feel about that?

And I tried not to comment but I have to say that I think its always best not to be complacent and go ahead and protect against rogue user actions. Regardless of how minute the consequences may seem. Am I wrong in saying that a person could append another SQL statement adding themself as a user of your DB? That little problem gets a lot bigger once tables are dropped and DB servers shutdown. I guarantee the evil intent is much more than what you underestimate. Okay thats my soapbox for today. I won't go overboard anymore. :D

chump2877
02-17-2006, 03:35 AM
Am I wrong in saying that a person could append another SQL statement adding themself as a user of your DB? That little problem gets a lot bigger once tables are dropped and DB servers shutdown.

I'd like to know how that is possible if the code is server side (if at all)....DB usernames, passwords, db names, table names, etc., are unaccessable, unless you think i'm passing this kind of vital data as hidden variables in my form...which i'm not

StupidRalph
02-17-2006, 03:39 AM
Yes sir I did believe you were passing the variables via your form.

fci
02-17-2006, 03:48 AM
I'd like to know how that is possible if the code is server side (if at all)....DB usernames, passwords, db names, table names, etc., are unaccessable, unless you think i'm passing this kind of vital data as hidden variables in my form...which i'm not

I'm pretty sure the mysql functions in php won't allow it, although there are other db functions that do allow it (basically just placing a semicolon within the call to run multiple queries at once). if you google for mysql injection (http://www.google.com/search?q=mysql+injection), you can see various examples of it.

chump2877
02-17-2006, 04:23 AM
I'm pretty sure the mysql functions in php won't allow it, although there are other db functions that do allow it (basically just placing a semicolon within the call to run multiple queries at once). if you google for mysql injection, you can see various examples of it.

I have magic_quotes_gpc set to ON in the php.ini, so this should prevent any kind of mysql injection, right? PhP is automatically escaping certain characters (in my POST input) that are inserted into a query, so that i don;t have to....these characters include single quotes and semicolons, as far as i know....

In the spirit of this post, which seems to have taken on a life of its own, LOL, I found this very good link, that i think all of you interested in this should check out: http://forums.devshed.com/php-development-5/everyone-must-read-security-notes-20525.html



Edit: OK, I see that neither the magic_quotes_gpc directive nor mysql_real_escape_string() escapes semi-colons...Is that a problem?

fci
02-17-2006, 06:26 AM
ew, magic_quotes_gpc is crap, you never want that on. and if you want it to escape a linefeed to insert into mysql, you'd need to first stripslashes then do mysql's escape function.


and:

SELECT user_id, user_name
FROM user
ORDER BY user_namel; INSERT INTO test (question) VALUES('wtf')
produces 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 '; INSERT INTO test (question) VALUES('wtf')' at line 1
SELECT user_id, user_name FROM user ORDER BY user_namel; INSERT INTO test (question) VALUES('wtf')

chump2877
02-17-2006, 08:33 AM
In other words, with that last example, you are illustrating that you dont need to escape semicolons, right? Because the query will fail?

And why exactly is "magic_quotes_gpc" crap? Does it not work? I have noticed that it does indeed work...I don;t seem to have problems with it...and by having it set on, you don;t have to use addslashes() or mysql_real_escape_string() every single time you want to insert user provided info into a database....

sounds like personal preference to me?

chump2877
02-17-2006, 09:01 AM
Alright, let's assume that magic_quotes_gpc is crap....you said this earlier:


I figured that's what he was doing, lazy way for anyone(not recursive):
Code:


$_POST = array_map('mysql_real_escape_string', $_POST);

although you should have a library to abstract all of this so you never have to call the ugly mysql_real_escape_string function.

So if I turn off magic_quotes_gpc, and use this line:


$_POST = array_map('mysql_real_escape_string', $_POST);

everytime I plan on inserting POST data into a MySQL db....This will sanitize everything in the POST and make it safe to use in a db query? Period? End of story?

fci
02-17-2006, 03:33 PM
it is not recursive so it's not "period/end of story" and it only escapes for mysql, imagine if you had oracle or something else where you didn't need that. The thing is, you generally don't want PHP itself to escape your data for you. you should be letting your mysql object/class do that for you.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum