0

So I am attempting to learn mysqli and do some database querying. I am using the class file that's found here Simple-Mysqli

I want to make sure that I do this right to prevent any mysql injection attempts but I am having a bit of trouble.

Here's my attempt at a query:

$id = '1776425144629';
$query = "SELECT * FROM article WHERE type = 'lnews' AND article_id = " . $id;
$query = $database->escape($query);
$results = $database->get_results( $query );
foreach( $results as $row )
{
    echo $row['headline'] .'<br />';
}

When I use the $database->escape() call that's in the class file (shown here)

 public function escape( $data )
 {
     if( !is_array( $data ) )
     {
         $data = $this->link->real_escape_string( $data );
     }
     else
     {
         //Self call function to sanitize array data
         $data = array_map( array( $this, 'escape' ), $data );
     }
     return $data;
 }

It errors out on me and shows the following:

 Query: SELECT * FROM article WHERE type = \'lnews\' AND article_id = 1776425144629
 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 '\'lnews\' AND article_id = 1776425144629' at line 1

So my question is, how can I pass some parameters to the sql statement, still using the escape or filter function to prevent any mysqli injections?

Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
MrTechie
  • 1,797
  • 4
  • 20
  • 36

2 Answers2

1

Ok, several issues here:

 $query = "SELECT * FROM article WHERE type = 'lnews' AND article_id = " . $id;

This line is the core of SQL Injection: Preventing SQL injection in a really simplified version van be described as: Do not concatenate your variables anymore. So no more

 $sometext . $someid;

or "$sometext $someid"

Not allowed, simple.

So what then? You will have to start using bind variables. Your query will then look like:

$query = "SELECT * FROM article WHERE type = 'lnews' AND article_id = ?";
$stmt=$mysqli->prepare($query);
$stmt->bind_param($id);

Also see http://php.net/manual/en/mysqli-stmt.bind-param.php for more details (*just skip to the examples at the bottom).

Second issue (I promised several issues):

$query = $database->escape($query);

This escapes the full query. Never use on a query, only on a variable (if it needs escaping: When using variable binding: Not needed: It is implicit).

Norbert
  • 6,026
  • 3
  • 17
  • 40
  • Ah, my misunderstanding on the escape process. Makes sense now. Question tho, doesn't the bind_param need to have an identifier of sorts ("s|i", $id)? And what about multiple params to bind? For example if lnews was bnews instead? Just do type = ? AND article_id = ? and then bind_params($type, $id)? – MrTechie Jun 06 '15 at 20:54
  • 1
    mysqli can just work with the order of the bind variables. So yes, your type=? and article_id=? is correct that way. – Norbert Jun 06 '15 at 20:56
1

The reason this is happening because you're escaping the actual query statement. PHP is escaping the quotations, hence the " \'lnews\' ". This will stop the query from working because you're executing this statement:

"SELECT * FROM article WHERE type = \'lnews\' AND article_id = 1776425144629"

What you want to do, is escape the values being passed through instead. Use the following instead:

$id = '1776425144629';
$id = $database->escape($id);
$query = "SELECT * FROM article WHERE type = 'lnews' AND `article_id` = '".$id."'";
$query = $database->$query;
$results = $database->get_results( $query );
foreach( $results as $row )
{
    echo $row['headline'] .'<br />';
}

This should work fine.

Hypernami
  • 138
  • 1
  • 8