Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A quotemeta() convenience function would be nice. #446

Closed
EvanCarroll opened this issue Sep 18, 2013 · 14 comments
Closed

A quotemeta() convenience function would be nice. #446

EvanCarroll opened this issue Sep 18, 2013 · 14 comments

Comments

@EvanCarroll
Copy link

When the user provides input there should be some way to sanitize it as a literal. This is provided in most languages as a core function quotemeta() in perl for instance.

It would be nice if the query object provided such a function. I can patch if it you're interested.

Then you could do something like

[ '%' + Query.quotemeta(req.params(s)) + '%' ]

For the parameterized input, and if the user supplied a '_' or '%' inside it's meant to be taken literally. In fact, I think it'd be good to suggest that of all req.params be sanitized in the docs.

@rpedela
Copy link
Contributor

rpedela commented Sep 19, 2013

Already exists, but currently undocumented.
#396

You should be using parameterized queries for user input though if you can. The Postgres server then handles everything for you, correctly.

@brianc
Copy link
Owner

brianc commented Sep 21, 2013

+1 for what @rpedela said. I believe he even supplied that functionality so another +1 for him for that. 😄

To elaborate a bit, if you concatenate strings of user input into your queries you need to stop immediately and use parameters instead. If you do use parameters there is no need to escape or clean them in your application because parameters are sent to the server and escaped properly within Postgres itself before your query is executed.

The following example is BAD:

pg.query('SELECT * FROM users WHERE name = ' + req.params.userName)

This works fine because postgres escapes it on the PostgreSQL server

pg.query('SELECT * FROM users WHERE name = $1', [req.params.userName])

@brianc brianc closed this as completed Sep 21, 2013
@brianc
Copy link
Owner

brianc commented Sep 21, 2013

Also: sorry if I come across as patronizing I'm definitely not trying to be - just want to be crystal clear on the use of parameters in case you weren't familiar with how they work. Better safe than sorry when it comes to security, ya know?

@EvanCarroll
Copy link
Author

@brianc that's not at all what I'm referring to. I know what parameterized queries do. People need a method not just to prevent SQL injection attacks (via planned queries), but to prevent SQL-METACHAR expansion.

In your example, if you use a placeholder and req.params.userName is set to '%foo' what do you expect to happen?

The % won't be modified because that's not what placeholders do, they just pre-plan the query. We need a method for '%foo' to be the literal '%foo'.

@rpedela
Copy link
Contributor

rpedela commented Sep 22, 2013

Can you give a specific example in the form of SQL?

@EvanCarroll
Copy link
Author

This is not difficult, @rpedela

    pg.query('SELECT * FROM users WHERE name = $1', [req.params.userName])

What happens if req.params.userName in the above example foobar%. That is to say what if the user name is a literal foobar% (with a percent sign %)? You have to provide some method to escape the SQL-METACHAR %. Otherwise, while injection isn't possible, that meta-character could make the query return multiple results which might not be expected if the name column is UNIQUE.

@rpedela
Copy link
Contributor

rpedela commented Nov 1, 2013

Not about difficulty. It is about me being confused on what you are talking about. Code solves that.

In your example, it will return rows only where name literally equals "foobar%". Instead are you referring to the LIKE operator? I think the code to escape a '%' or '_' is pretty trivial, but of course it is up to @brianc.

@aashidham
Copy link

@brianc @rpedela I'm curious about the state of this issue, since it has been a few years. Is this issue resolved?

@rpedela
Copy link
Contributor

rpedela commented Jan 4, 2017

OP never made it clear what was wrong. What related issue are you having and can you provide example code? If you just need escaping then I recommend pg-format which I wrote.

@aashidham
Copy link

aashidham commented Jan 4, 2017 via email

@rpedela
Copy link
Contributor

rpedela commented Jan 4, 2017

What is the "pg mom module"?

@rpedela
Copy link
Contributor

rpedela commented Jan 4, 2017

If you meant "pg module" and the "mom" part is a typo, then pg does not escape by default. It has parameterized queries where the query and parameters are sent separately over the wire and then the server inputs those parameters into the query in a safe manner. pg also has escapeLiteral and escapeIdentifier functions which only work for strings (I ported them from Postgres C code).

pg-format handles escaping for all JS and Node types and it can be used in situations where parameterized queries don't work. For example, let's say you wanted to dynamically specify the list of columns in a SELECT. You must escape each column name as an identifier to protect against SQL injection, but that is not possible with parameterized queries.

// SELECT a, b, c FROM my_table
var pg = require('pg');
var pgFormat = require('pg-format');

var columns = [ 'a', 'b', 'c' ];

// using pg
var escapedColumns = [];
for (var i = 0; i < columns.length; i++) {
    escapedColumns.push(pg.escapeIdentifier(columns[i]));
}
var sql = 'SELECT ' + escapedColumns.join(',') + ' FROM my_table';

// using pg-format
var sql = pgFormat('SELECT %I FROM my_table', columns);

This is a trivial example, but pg-format handles a lot of the logic you would have to write yourself so it simpler and correct.

@aashidham
Copy link

aashidham commented Jan 4, 2017 via email

@rpedela
Copy link
Contributor

rpedela commented Jan 4, 2017

That is a parameterized query and yes you would be protected. If you wanted to use pg-format instead, here is the equivalent query.

var pgFormat = require('pg-format');
var arr = ['bob', 'cat'];

var sql = pgFormat('SELECT col1 FROM containers_hist WHERE ipaddr = ANY (ARRAY[%L])', arr);

// alternative version
var sql = pgFormat('SELECT col1 FROM containers_hist WHERE ipaddr IN (%L)', arr);

pool.query(sql);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants