-3

I have this code:

<?php
$table = $_GET ["table"];
$query = "SELECT 1 FROM $table";
$st = $pdo->prepare($query);
$st->execute();

This is not the real code, but it is an example to get the idea.

If I make:

hacked.php?table=users;DROP TABLE users;

It will work, cause it is not correctly escaped.

However, if I want to update information like this:

hacked.php?table=users; UPDATE users SET name="abc" WHERE name="def";

It will not work, cause since it is escaped, pdo will convert the query to

SELECT 1 FROM users; UPDATE users SET name=\"abc\" WHERE name=\"def\";

and obviously it fails.

Is there anyway to make this query works?

EDIT 1

We have one guy in our team only devoted to check my code and hacked it. So I want to be ready if this can be in some way accomplished.

EDIT 2

I was already read this: Are PDO prepared statements sufficient to prevent SQL injection? but it really did not answered my question. However it gave me a way to go through. And the solution of @duskwuff was the same I came to. So, for the admins, if this should be removed or marked as a duplicate is ok. But I insist that this can be helpful for someone to know how pdo prepared can be hacked.

Community
  • 1
  • 1
Cito
  • 1,659
  • 3
  • 22
  • 49
  • `prepare` does not do magic that makes your code safe. If you have built a SQL command with outside data, you are in danger. – Andy Lester Sep 05 '13 at 04:00
  • One don't need a "devoted team member" to tell that solution is just awful. Even if it's your own code, it's awful shame to have a table name from GET. Means **database structure is flawed** in the first place, beside that approach which is **not actually using** prepared statements. And still helping to hack a fellow webmaster is a shame. – Your Common Sense Sep 05 '13 at 06:14
  • And still tales about "devoted team member" are cheap talks. To prove this code can be hacked, a working DROP TABLE you have already **is enough.** – Your Common Sense Sep 05 '13 at 06:18
  • @YourCommonSense. Really... You don't get the point. I'm not using get in the real code. I'll not paste it here. This is only an example to gave you guys an idea of what I wanted to accomplished. Thanks for your good-for-nothing comments. – Cito Sep 05 '13 at 10:54
  • 1
    possible duplicate of [Are PDO prepared statements sufficient to prevent SQL injection?](http://stackoverflow.com/questions/134099/are-pdo-prepared-statements-sufficient-to-prevent-sql-injection) – andrewsi Sep 05 '13 at 14:58
  • 1
    http://php.net/manual/en/mysqli.quickstart.prepared-statements.php Just do like this and you'll be safe, you're not binding the parameters... – matheussilvapb Jul 29 '14 at 21:09
  • Thanks for your comment @matheussilvapb however the problem is you cannot bind the name of the table nor the name of a field. However, I came to notice doing it with chr(1) it can be hacked. So in the end, the solution was to use an array with tables and compare if the name of the table was in that array or not, instead of using it directly – Cito Jul 30 '14 at 13:43

2 Answers2

3

I think you are asking the wrong question. If you have code that is even remotely similar to this, then you have a huge problem with the way you're writing code... and probably with the way you're conceptualizing the problem that you need to solve, or you're working from a very bad design.

If, for some reason, you have a need for anything about the design of your database to be passed in on a URL query string or an http post, and if, for some reason, you think executing an unescaped query is the approach you need... then whatever you're doing, you're doing it wrong.

If, by some remote chance, you actually have a need to pass the name of a table to a web page, then the very least you must do is compare the input value to some kind of static structure to see if the input value is in the list... and then use the value from the list, or from something static, never from the input.

Simplistically something as primitive as the following would be a far superior approach, though arguably it is a bad design if table names, column names, or any database internals ever need to go out into browser-land.

$table = $_GET ["table"];
IF ($table == "users")
{
   $query = "SELECT 1 FROM users;"
}
ELSEIF ($table == "points")
{
   $query = "SELECT 1 FROM points;"
}  

...

Michael - sqlbot
  • 169,571
  • 25
  • 353
  • 427
  • Thanks for your answer. I'm not doing it like that. However, I've one part of the system that can be used like that... Not the exact same code, but only to be understood I wrote that example. – Cito Sep 05 '13 at 03:40
  • 2
    so you actully believe the OP? –  Sep 05 '13 at 03:40
3

It will not work, cause since it is escaped, pdo will convert the query to

SELECT 1 FROM users; UPDATE users SET name=\"abc\" WHERE name=\"def\";

This is incorrect! PDO does not perform escaping on text that is interpolated into queries, as it has no awareness of what text was interpolated. What you're seeing is the result of PHP's deprecated magic_quotes feature adding backslashes to the content of request variables (like $_GET and $_POST). Even if this is enabled, it can be trivially avoided in a query like this one by using non-quoted constructs such as:

SELECT 1 FROM users; UPDATE users SET name = CHAR(97,98,99) WHERE name = CHAR(100,101,102)

(CHAR() is a MySQL function which constructs a string from a list of character code values. If you're using some other database, an equivalent function probably exists.)

Interpolating unescaped content directly into a query is never safe. Don't do it.

  • I've marked your answer as a solution since it was what I've found. Char can be used to hack the system. But you are incorrect. I'm not using actual GET, the request is made by post, and the server has magic_quotes removed since it throws a fatal error at the newer version of PHP and I know it is very insecure to use it in production. So that is because of pdo preparing the query. Still, your solution works :) – Cito Sep 05 '13 at 03:58
  • The only thing that would cause the effects you're describing in your question would be `magic_quotes`. Either it is enabled after all, or something else in your application is replicating its effects. –  Sep 05 '13 at 04:13