Page 1 of 1

sprintf, does not like my code - only sometimes???

Posted: Thu Jun 29, 2006 4:46 pm
by redrob
I've got a PHP file that processes the results of a user submitted form

I go over every field that could be passed in from the form, and build an SQL query
to insert the necessary data into a database.

Before I attempt to access any of the fields, I check if they were passed by the
user's browser with isset(). For example, here is a snippet of code I use to check
for a field, and add it to my sql string if it exists.

Code: Select all

if ( isset($_REQUEST['girdle2']) ) {
	$strSQL = sprintf($strSQL . ", girdle2 = '%s'",
				   mysql_real_escape_string($_REQUEST['girdle2'])
					 );
}

Seems OK to me, and it works when I test it. BUT about 1 in 100 times when a user
submits the form I will get an error in the PHP errorlog - and I don't get the data
that the user sent in due to the error :-(

Here is the error I will sometimes get:

[Thu Jun 29 17:02:49 2006] [error] [client 69.63.192.11] PHP Warning: sprintf(): Too few arguments in /home/httpd/vhosts/mysite.com/httpdocs/quote/stage2_save.php on line 56, referer: http://www.mysite.com/quote/

So, the question then is how can I be getting an error in sprintf? I'm only accessing
a single variable, and I just confirmed one line before that the variable does exist.

Any idea what is going on here?

Posted: Thu Jun 29, 2006 4:57 pm
by daedalus__
It says too few arguements, which means there is another % something int here somewhere.

Perhaps a user is submitting %s, %d, or one of the other wildcard thingies?

Posted: Thu Jun 29, 2006 5:08 pm
by redrob
Daedalus- wrote:It says too few arguements, which means there is another % something int here somewhere.
I assumed that it was saying to few arguments because the variable I was using did not exist. How can another %s be getting in there? Users are actually selecting options from a drop down list.

Posted: Thu Jun 29, 2006 5:43 pm
by RobertGonzalez
If you have a '%' sign anywhere in the string it tries to evaluate it. Something like a SQL wildcard (WHERE this = '%something%') could really cause some issues.

Posted: Thu Jun 29, 2006 6:25 pm
by redrob
I see... so I would be much safer to do something like this:

Code: Select all

if ( isset($_REQUEST['girdle2']) ) {
	$strSQL =. sprintf(", girdle2 = '%s'",
				   mysql_real_escape_string($_REQUEST['girdle2']));
}
(not sure of that exact =. or .= syntax) this way I am in more control as to exactly what is being passed into the sprintf function. I try that and see what happens over the next week or so as people submit the form.

Posted: Thu Jun 29, 2006 6:57 pm
by RobertGonzalez
If you have a wildcard in your SQL, you will still get the error doing it the way you are doing it. You may just want to clean the var before the query, the just add the var to the query rather than running it through sprintf().

Oh yeah, concatenating strings is .= ...

Code: Select all

<?php
$string = 'I love elephants';
$string .= ' and they love me';

echo $string;
?>

Posted: Thu Jun 29, 2006 8:38 pm
by redrob
I am running it through sprintf() to prevent script injection attacks. Even if I have choices from a dropdown list someone can still inject something into the sql manually. I've had a number of attacks against systems I've done by people using script injections so I am trying to cover my butt with sprintf().