Have I sanitized this correctly?

[chrisguk]

Hi,

Brad gave me a suggestion to sanitize my input to prevent SQL injection. I have made some adjustments below, is this correct?

function sanitize($sql)
{
$sql = trim($sql);

if(get_magic_quotes_goc())
{
$sql = stripslashes($sql);

$sql = mysql_real_escape_string($sql);

return $sql;
}

$sql = $mysqli->query("SELECT * FROM url WHERE page = '" . $page . "' LIMIT 1");

if ($mysqli->connect_errno) {
    printf("Connect failed: %s\n", $mysqli->connect_error);
    exit();
}

if ($sql->num_rows > 0) {
    while ($row = $sql->fetch_object())
    {
        $title = $row->title;
        $description = $row->descr;
        $keywords = $row->keywords;

}

} else {


$title = 'Set Default Title';
$description = 'Set Default Description';
$keywords = 'Set Default Keywords';

}

}

[Weedpacket]

1. You don't do anything in that code to sanitise [font=monospace]$page[/font].

2. You don't use that [font=monospace]sanitize()[/font] function anywhere in that code.

3. Even if you were using the function...

   1. There is no such function as [font=monospace]get_magic_quotes_goc[/font]. If you mean [man]get_magic_quotes_gpc[/man], note that as of PHP 5.4 it doesn't do anything.

   2. [man]mysql_real_escape_string[/man] is deprecated, especially since you're using MySQLi anyway.

   3. [man]stripslashes[/man] does the opposite of what [man]mysql_real_escape_string[/man] would do. It would be used if you had something like the (no longer functional) magic quotes and were not planning to put user-supplied data in a database query.

[chrisguk]

Oh sigh 🙁

Would you be kid enough to show me how to sanitize it so I can learn more and build on it?

[Bonesnap]

chrisguk;11020869 wrote:

Oh sigh 🙁

Would you be kid enough to show me how to sanitize it so I can learn more and build on it?

There's really two different ways you can approach it. One, you can use mysqli's real_escape_string, such as:

$page = $mysqli->real_escape_string($page);

Or you can use prepared statements, such as:

$stmt = $mysqli->prepare("SELECT * FROM url WHERE page = ? LIMIT 1");
$stmt->bind_param('s', $page);

[bradgrafelman]

Bonesnap;11020871 wrote:

One, you can use mysqli's real_escape_string, such as:

$page = $mysqli->real_escape_string($page);

... for string data only. If you had a numeric ID value being passed around via the URL, for example, you would not want to the string escaping function. Instead, you would want to do something such as casting the user-supplied data to an (int), use the [man]intval/man function, build your query strings using [man]sprintf/man (enabling you to use a 'd' or 'u' format specifier), etc.

In other words, if you don't use prepared statements, there is no single technique that can be followed to sanitize all types of data.

[Bonesnap]

bradgrafelman;11020873 wrote:

... for string data only. If you had a numeric ID value being passed around via the URL, for example, you would not want to the string escaping function. Instead, you would want to do something such as casting the user-supplied data to an (int), use the [man]intval/man function, build your query strings using [man]sprintf/man (enabling you to use a 'd' or 'u' format specifier), etc.

In other words, if you don't use prepared statements, there is no single technique that can be followed to sanitize all types of data.

That is true; mysqli_real_escape_string is intended for string data.

I wrote my example as a string since based on the OP's code $page is a string.

[chrisguk]

here we go, I have been reading about PDO, how about this solution?

$DBH = new PDO("mysql:host=$server;dbname=$dbname, $user, $pass");

// Search database for pages matching the current 
// page and display the title, description and keywords

$page = isset($_GET['page']) ? $_GET['page'] : 'home';


//$page = $mysqli->real_escape_string($page);

$stmt = $DBH->prepare("SELECT * FROM url WHERE page = '" . $page . "' LIMIT 1");
$stmt->bindParam('$page', $_GET['§page'], PDO::PARAM_INT);
$stmt->execute();

[laserlight]

This looks wrong:

$stmt = $DBH->prepare("SELECT * FROM url WHERE page = '" . $page . "' LIMIT 1");
$stmt->bindParam('$page', $_GET['§page'], PDO::PARAM_INT);

I would expect:

$stmt = $DBH->prepare("SELECT * FROM url WHERE page = :page LIMIT 1");
$stmt->bindParam(':page', $_GET['§page'], PDO::PARAM_INT);

or:

$stmt = $DBH->prepare("SELECT * FROM url WHERE page = ? LIMIT 1");
$stmt->bindParam(1, $_GET['§page'], PDO::PARAM_INT);

[Weedpacket]

For another thread touching on the subject of sanitising database input, see this one from April:

http://board.phpbuilder.com/showthread.php?10384231-RESOLVED-Values-not-going-in-DB

[chrisguk]

Weedpacket;11020969 wrote:

For another thread touching on the subject of sanitising database input, see this one from April:

http://board.phpbuilder.com/showthread.php?10384231-RESOLVED-Values-not-going-in-DB

Hi Weed sorry about that, since that post I fell very ill with a brain issue so my memory isnt what is used to be. Essentially I am trying to learn all over again.