-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Comments
Already exists, but currently undocumented. You should be using parameterized queries for user input though if you can. The Postgres server then handles everything for you, correctly. |
+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]) |
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? |
@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 |
Can you give a specific example in the form of SQL? |
This is not difficult, @rpedela
What happens if |
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 |
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. |
How is this different from what is used in pg mom module directly? I thought that format syntax did this escaping by default.
… On Jan 4, 2017, at 12:31 PM, Ryan Pedela ***@***.***> wrote:
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.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
What is the "pg mom module"? |
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 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. |
I see. That was a typo on my end, apologies, I meant to say npm.
What about in this scenario:
//using pg
var arr = ['bob', 'cat'] //arr is populated from user input
pool.query('select col1 from containers_hist where ipaddr= ANY ($1)', arr)
Is pg protecting me from user input in this case?
…On Wed, Jan 4, 2017 at 3:53 PM, Ryan Pedela ***@***.***> wrote:
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 <https://github.com/datalanche/node-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_tablevar pg = require('pg');var pgFormat = require('pg-format');
var columns = [ 'a', 'b', 'c' ];
// using pgvar 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-formatvar sql = pgFormat('SELECT %I FROM my_table', columns);
This is a trivial example, but pg-format
<https://github.com/datalanche/node-pg-format> handles a lot of the logic
you would have to write yourself so it simpler and correct.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#446 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAvvSWavM0SLHlm0vM-azJruJx1utOoYks5rPAbOgaJpZM4BAvmA>
.
|
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); |
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.
The text was updated successfully, but these errors were encountered: