0

Let's say I have a website which sells instruments. I am using PHP and MySQL in order to dynamically build the products page labelled products.php.

Now my table has multiple columns, such as ParentCategory and the like. Pressing on the products tab would take you to category.php, where there will be some categorical buttons you can press on, which will take you to products.php and only show you the relevant products.

Now here enlies the problem. To retrieve the items from the database, we need to set a query like

SELECT * FROM productlist WHERE ParentCategory=1

From the category page, the buttons are wrapped in a form, and each button submits a variable called query using POST. The value would be something like ParentCategory=1.

The full SQL query, with the tablename, is hidden, however the multiple subqueries that the buttons have to submit, such as ParentCategory=1 or ParentCategory=2 are displayed in the HTML of the page itself.

What I want to know is, is this a massive security flaw? As in, could a malicious user exploit such information for hacking purposes? Personally I couldn't think of a way a user could exploit the site after only knowing its column names, but I'm also only starting out in web development.

If that's the case, then what's the safest or universal way that these queries are passed? I would have thought GET but that lengthens the URL and opens the problem of injection up, since any values can be passed.

ENBYSS
  • 819
  • 1
  • 10
  • 22
  • 1
    `GET` isn't any safer from injection than `POST` - any modern browser has a bundled debugging toolset that allows you to manipulate `POST` data. As for whether it's a security flaw - that rather depends on what you're doing with those variables and how you're building the query - we can't really answer that without seeing some code. – CD001 May 15 '18 at 09:26
  • Can you show how you're using the POST parameters in your code? If you're using proper SQL-injection protection, it shouldn't be an issue. – Barmar May 15 '18 at 09:27
  • if all you're exposing is things like `ParentCategory=1` then it's not massive. The massive flaw is if you're using what the user sends verbatim e.g. if a user sends `ParentCategory=1 OR 1=1` – apokryfos May 15 '18 at 09:30
  • Oh no, the actual query is never written by the user, it's sent once a button is clicked. Apart from that there's no injection protection in place so far – ENBYSS May 15 '18 at 10:47

3 Answers3

4

is this a massive security flaw

YES.

It's not about showing SQL on the client side, it's that you're taking SQL from the browser as is and execute it on your server. That's SQL injection by definition. The browser isn't really limited to the SQL snippets you put into your HTML; the browser (read: any HTTP client) could send any freeform SQL back to your server. If your server just executes any given SQL from any arbitrary client, you're dead.

Take some defined parameters and translate them into SQL. E.g.:

if ($_GET['order'] == 'product') {
    $sql .= 'ORDER BY products.id';
}
deceze
  • 510,633
  • 85
  • 743
  • 889
  • The question doesn't actually state that they're using the column names directly from the form and injecting them into an SQL query... just that the column names are being exposed. You may well be right that that's what's happening - but the question could use some clarity... and showing some code. – CD001 May 15 '18 at 09:29
  • I'm simply assuming that's what's happening reading between the lines, yes. – deceze May 15 '18 at 09:30
  • Just now realised that people can modify a POST query, so... thanks a lot for helping! – ENBYSS May 15 '18 at 10:49
1

That looks like a bad idea to me, if you're planning on pasting the text from the form directly onto the end of your "where" clause. If you're doing a POST with some form value set to "ParentCategory=1", someone could as easily do SQL injection via a POST as via a GET (see Is SQL Injection possible with POST? for more info).

A better way to do this is something like this:

  1. Your user selects the parent category from a list.
  2. Your form sends back the category via a get/post parameter ("ParentCategory=2", or "ParentCategory=Dog Walking", or something).

  3. Your PHP Code constructs a query that maps your form parameter to the name of the database column:

    SELECT * FROM productlist WHERE parent_category=2

  4. Send the query constructed in PHP to the database.

Matt Hartfield
  • 22
  • 4
-1

First of all to be clear, what you've called "small part of sql query" called http query string, and its passed through GET request, and available in PHP via $_GET array, so it is not technically part of your SQL query until you pass it to your SQL query.

To answer your question, - It is safe if you sanitize variables, for PHP you have to use mysqli_real_escape_string function, take a look here https://secure.php.net/manual/en/mysqli.real-escape-string.php

If you have to use PDO then use PDO::prepare which is automatically quote variables content to prevent SQL injection. https://secure.php.net/manual/en/pdo.prepare.php

noroot
  • 142
  • 2
  • How exactly would you sanitise/prepare an *SQL snippet* like `ParentCategory=1`…? – deceze May 15 '18 at 09:32
  • `$query = sprintf("SELECT * FROM categories WHERE ParentCategory id = %s",mysqli_real_escape_string($_GET['ParentCategory'])); ` – noroot May 15 '18 at 09:39
  • If `$_GET['ParentCategory']` is `1 OR 1=1` then it doesn't need escaping and is still allowing users to get all the data in a table. The problem being that 1 is a number which is not quoted so everything else is open. As the name suggests `mysqli_real_escape_string` escapes strings. This is why when using prepared statements you need to specify the data type – apokryfos May 15 '18 at 09:44