[SOLVED] Should You mysql_real_escape_string ALL form data?

[limitphp]

Even checkboxes?

 

I have a form that has a bunch of checkboxes that are genres people select.  Eventually, the value of those checkboxes ends up in a sql statement.

 

Would I need to real_escape the checkbox values?

I assume, someone could build a fake form and submit it to my page and have the name be the same and the value would be a malicious sql injection.

 

 

 

 

[Maq]

Yeah should use it for anything that goes into your database.  It can't hurt anyway...

[limitphp]

Yeah should use it for anything that goes into your database.  It can't hurt anyway...

 

My genres checkboxes are an array.

ex)

<input ID="genre" type="checkbox" name="genre[]" value="Country" />

<input ID="genre" type="checkbox" name="genre[]" value="Rock" />

etc...

 

So, $_POST['genre'] returns an array.  Before I can mysql_real_escape it, I have to display the contents of the array.

 

Eventually the contents have to be in the form: 'rock','country'

 

seperated by commas and have single quotes around them.  they have to be this way because they are used in a query that requires them to be in this form:

(simplified version of the query):

select * from songs where genre IN ($genres)

 

Right now, I get them in that form by doing this:

$genres = "'" . implode("','", $_POST['genre']) . "'";

 

Someone on this board helped me come up with that; I can't remember who right now.

 

My problem is, I can't real_escape it before I do that because its still an array, and I can't real_escape it after that, because I need those single quotes or it will error on the query.

 

 

[premiso]

<?php
array_walk($_POST['genre'], "mysql_real_escape_string");
$genres = "'" . implode("','", $_POST['genre']) . "'";
?>

 

Should apply that function to each element of $_POST['genre']

[PFMaBiSmAd]

mysql_real_escape_string() as its' name implies should be used on all string data that is placed into a query (would be enclosed in single-quotes in the query) that could contain special characters that would break a query (or allow sql injection.)

 

For numeric data (anything that is not a string), either validate that it is numeric or cast the value as a number to avoid sql injection.

[limitphp]

<?php
array_walk($_POST['genre'], "mysql_real_escape_string");
$genres = "'" . implode("','", $_POST['genre']) . "'";
?>

 

Should apply that function to each element of $_POST['genre']

 

Doesn't array_walk just return a 1 or 0?  It won't actually replace anything will it?

 

edit: oh by the way, premiso, I figured out what was wrong with the mod rewrite messing up the cookies.....it turns out, you can't set the path if you have the expire value equal to 0.

Don't ask me why, I have no idea....but once I changed it from 0 to an actual time in the future it worked!

 

[premiso]

Description

bool array_walk ( array &$array , callback $funcname [, mixed $userdata ] )

 

Applies the user-defined function funcname to each element of the array array.

 

array_walk() is not affected by the internal array pointer of array . array_walk() will walk through the entire array regardless of pointer position.

 

It returns a bool, but as you notice I do not set it to anything. It goes through the array, by reference, and applies that to every element of the array. The data will be escaped.

 

The example from the man page:

Examples

 

Example #1 array_walk() example

<?php
$fruits = array("d" => "lemon", "a" => "orange", "b" => "banana", "c" => "apple");

function test_alter(&$item1, $key, $prefix)
{
    $item1 = "$prefix: $item1";
}

function test_print($item2, $key)
{
    echo "$key. $item2<br />\n";
}

echo "Before ...:\n";
array_walk($fruits, 'test_print');

array_walk($fruits, 'test_alter', 'fruit');
echo "... and after:\n";

array_walk($fruits, 'test_print');
?>

 

The above example will output:

Before ...:
d. lemon
a. orange
b. banana
c. apple
... and after:
d. fruit: lemon
a. fruit: orange
b. fruit: banana
c. fruit: apple

[limitphp]

Description

bool array_walk ( array &$array , callback $funcname [, mixed $userdata ] )

 

Applies the user-defined function funcname to each element of the array array.

 

array_walk() is not affected by the internal array pointer of array . array_walk() will walk through the entire array regardless of pointer position.

 

It returns a bool, but as you notice I do not set it to anything. It goes through the array, by reference, and applies that to every element of the array. The data will be escaped.

 

The example from the man page:

Examples

 

Example #1 array_walk() example

<?php
$fruits = array("d" => "lemon", "a" => "orange", "b" => "banana", "c" => "apple");

function test_alter(&$item1, $key, $prefix)
{
    $item1 = "$prefix: $item1";
}

function test_print($item2, $key)
{
    echo "$key. $item2<br />\n";
}

echo "Before ...:\n";
array_walk($fruits, 'test_print');

array_walk($fruits, 'test_alter', 'fruit');
echo "... and after:\n";

array_walk($fruits, 'test_print');
?>

 

The above example will output:

Before ...:
d. lemon
a. orange
b. banana
c. apple
... and after:
d. fruit: lemon
a. fruit: orange
b. fruit: banana
c. fruit: apple

 

I did read through that example, but its pretty complicated for me.  Also, I have a clean function I use:

<?php
function clean($value, $type)
{
if ($type=="sql")
{
	// Stripslashes
	if (get_magic_quotes_gpc())
	  {
	  $value = stripslashes($value);
	  }
	  $value = mysql_real_escape_string($value);
}elseif ($type=="html")
{
	$value = htmlspecialchars($value, ENT_QUOTES);
}
return $value;
}

 

Unfortunately, it needs to be passed an argument $type.  How do you pass an argument ('sql') to clean() using array_walk?

Like this:

array_walk($_POST['genre'], "clean","sql");

 

sorry, this is really confusing me.

 

So, I could just do it like this:

 

array_walk($_POST['genre'], "clean","sql");
$genres = "'" . implode("','", $_POST['genre']) . "'";

 

And that will clean it first and then add all the stuff to it?

 

 

 

 

[gevans]

If you want to clean everything in $_POST using your function just use foreach;

 

<?php
foreach($_POST['genre'] as $key=>$value){
$_POST['genre'][$key] = clean($value, $type);
}
$genres = "'" . implode(",", $_POST['genre']) . "'";

[limitphp]

If you want to clean everything in $_POST using your function just use foreach;

 

<?php
foreach($_POST['genre'] as $key=>$value){
$_POST['genre'][$key] = clean($value, $type);
}
$genres = "'" . implode(",", $_POST['genre']) . "'";

 

Awesome...thank you!

 

sorry for all the questions, but arrays really confuse me.....

 

[limitphp]

Just tested it out...

foreach($_POST['genre'] as $key=>$value){

$_POST['genre'][$key] = clean($value, $type);

}

$genres = "'" . implode(",", $_POST['genre']) . "'";

 

it works perfect.....

 

so basically, if some jackball puts some single quote in there, and tries to inject some malicious code, it will replace it now with a \' and the query will mess up and they will get an ugly error on my page.....

 

hopefully.....

 

thanks guys!

[PFMaBiSmAd]

As already mentioned above, if the data is numeric, such as in -

 

SELECT * FROM your_table WHERE id = $id

 

A UNION query can be included as part of the $id that does not involve quotes (it turns out hex data is treated as a string and you don't need to use quotes) where anything in your table can be output, such as user names and hashed password values. Here is a real live example where this was used - http://www.codingforums.com/showthread.php?t=151150

[limitphp]

As already mentioned above, if the data is numeric, such as in -

 

SELECT * FROM your_table WHERE id = $id

 

A UNION query can be included as part of the $id that does not involve quotes (it turns out hex data is treated as a string and you don't need to use quotes) where anything in your table can be output, such as user names and hashed password values. Here is a real live example where this was used - http://www.codingforums.com/showthread.php?t=151150

 

Unfortunately its not numeric...I store the genres in the songs table as their respective names (rock, blues, etc)....

here's the actual query:

$queryVote = "SELECT vote.artistID, vote.songID, vote.Date, artist.artistName, artist.artistNameHash, songs.songName, songs.songNameHash, songs.genre, COUNT(vote.songID) FROM vote, artist, songs WHERE vote.artistID=artist.artistID AND vote.songID=songs.songID AND songs.genre IN ($genres) AND vote.date > NOW()-INTERVAL $time DAY GROUP BY vote.songID ORDER BY COUNT(vote.songID) DESC LIMIT $offset, $rowsperpage";

$resultVote = mysql_query($queryVote) or die (mysql_error());

 

Even if I stored the genres as numbers in the songs table...they would still need quotes around them for the query.