3

Will my queries be safe from SQL injections if I use plpgsql functions only to validate identifiers and values, instead of executing queries.

Here are the example functions:

CREATE OR REPLACE FUNCTION validate_identifier(identi TEXT)
  RETURNS TEXT AS
$func$
BEGIN
  RETURN quote_ident(identi);
END;
$func$ LANGUAGE plpgsql;


CREATE OR REPLACE FUNCTION validate_value(val TEXT)
  RETURNS TEXT AS
$func$
BEGIN
  RETURN quote_literal(val);
END;
$func$ LANGUAGE plpgsql;

then i.e. simply use query:

SELECT * FROM some_table WHERE validate_identifier('user_input_identifier') = validate_value('user_input_value');

Is this query safe from SQLi, when user_input_identifier and user_input_value are replaced by actual user input, which might be an SQLi attempt?

  • Besides not being safe, the proposed technique doesn't work at all, because the SQL engine will not interpret the result of validate_identifier() as an identifier: it's just a string and it won't correlate it to the table in any way. IOW in most cases it will do SELECT * FROM some_table WHERE false – Daniel Vérité Jan 8 '18 at 7:57
4

Is this query safe from SQLi, when user_input_identifier and user_input_value are replaced by actual user input, which might be an SQLi attempt?

The whole sql injection vulnerability is about sanitized input, depending on how you're getting the inputs to the sanitizing function it may not even be safe to use them. In this case, I'll assume you're calling the SELECT statement from a client-library. In such a case, it's

  • no safer than if you just called the functions directly.
  • less efficient
  • still vulnerable to sql injection

The important thing to be safe is that you're binding the values to the input of the function with placeholders.

If your code looks like this,

k = sprintf("
  SELECT *
  FROM some_table
  WHERE validate_identifier(%s) = validate_value(%s)
");

You're not safe. If it looks like this,

k = sprintf("
  SELECT *
  FROM some_table
  WHERE quote_ident(%s) = quote_value(%s)
");

You're not safe. If it looks like this,

k = sprintf("
  SELECT *
  FROM some_table
  WHERE \"%s\" = '%s'
");

You're not safe. And, for all the same reasons. If it looks like this,

k = "
  SELECT *
  FROM some_table
  WHERE col = ?
";
k.exec($foo);

You're safe. If you don't know col at compile time, for instance if you want to safely do ? = ?, the library will either prefetch the escaped version by running something like this,

k = "SELECT quote_ident(?);"

Or it'll simply use a client-side version (which usually binds to libpq) thus saving a trip to the server.

So in answer to your question

Is it efficient to use plpgsql functions only to validate identifiers and values?

It's certainly less efficient, and depending on how you're providing those functions their values, potentially not safe either.

  • So this means that if the whole query is executed within a function with the identifier and value passed in as a parameter later on, then I'm safe? Right? – Qualphey Jan 6 '18 at 21:02
  • A query can't be prepared without all the identifiers. That's part of preparation. You can't select from a table if you don't know what table you're selecting from. When a statement is prepared it's know to be valid. From that point you know you're safe with any values. The only question is how are you providing your identifiers and what does your library do. You showed SELECT * FROM some_table WHERE validate_identifier('user_input_identifier') = validate_value('user_input_value') That's not enough to tell if you're safe. We need to see how it actually looks on the app. – Evan Carroll Jan 6 '18 at 21:05
  • But the prepared statements give a performance increase, not SQLi protection, am I right? – Qualphey Jan 6 '18 at 21:21
  • No, it gives sql injection protection too because values are known to be values which protects you from sql injection. – Evan Carroll Jan 6 '18 at 21:23
  • 1
    @Qualphey, a bit off topic but prepared statements does not increase performance per se. It makes it possible to reuse statements without recompiling, so it reduces cpu utilization and memory consumption. This is often beneficial for performance, especially for trivial OLTP queries. However, with skew data and/or big complex report queries the pre-compiled statement may fit the current parameters badly, so it may be beneficial performance-wise to recompile the statement every time it is executed. – Lennart Jan 6 '18 at 23:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.