Avoid SQL injection while inserting a mix of hard-coded and variable values?

Question

I'm writing database queries with pg-promise. My tables look like this:

                                    Table "public.setting"
│ user_id          │ integer           │ not null                
│ visualisation_id │ integer           │ not null  
│ name             │ character varying │ not null                                  

                                    Table "public.visualisation"
│ visualisation_id │ integer           │ not null                
│ template_id      │ integer           │ not null

I want to insert some values into setting - three are hard-coded, and one I need to look up from visualisation.

The following statement does what I need, but must be vulnerable to SQL injection:

var q = "INSERT INTO setting (user_id, visualisation_id, template_id) (" +
        "SELECT $1, $2, template_id, $3 FROM visualisation WHERE id = $2)";
conn.query(q, [2, 54, 'foo']).then(data => {
    console.log(data); 
});

I'm aware I should be using SQL names, but if I try using them as follows I get TypeError: Invalid sql name: 2:

var q = "INSERT INTO setting (user_id, visualisation_id, template_id) (" +
        "SELECT $1~, $2~, template_id, $3~ FROM visualisation WHERE id = $2)";

which I guess is not surprising since it's putting the 2 in double quotes, so SQL thinks it's a column name.

If I try rewriting the query to use VALUES I also get a syntax error:

var q = "INSERT INTO setting (user_id, visualisation_id, template_id) VALUES (" +
        "$1, $2, SELECT template_id FROM visualisation WHERE id = $2, $3)";

What's the best way to insert a mix of hard-coded and variable values, while avoiding SQL injection risks?

to use query as value, take it into brackets like a subquery, eg `"$1, $2, (SELECT template_id FROM visualisation WHERE id = $2), $3)"` — Vao Tsun, Aug 24 '17 at 09:58
I think @VaoTsun is correct here. And SQL Names got nothing to do here, since you are not using dynamic column names. — vitaly-t, Aug 24 '17 at 10:30
Thanks both. In the first example, couldn't the user supply some SQL in place of `$1` and potentially achieve SQL injection, because the inner query uses dynamic column names? (I'm checking existing code for vulnerabilities.) — Richard, Aug 24 '17 at 10:40
@Richard I don't think so, as `$1` represents a simple value. If you pass it any string, it will be properly escaped as a string, no injection will be possible. — vitaly-t, Aug 24 '17 at 10:50
Interesting, thanks! So in a query like `SELECT $1, $2, template_id, $3 FROM visualisation WHERE id = $2` there's no need to use SQL names? Why then are they needed at all? — Richard, Aug 24 '17 at 10:58
@Richard you would need to use SQL Names there, in a simple SELECT. But your example is an INSERT that uses SELECT, which is a different story. — vitaly-t, Aug 24 '17 at 11:13

score 2 · Accepted Answer · edited Aug 24 '17 at 12:41

Your query is fine. I think you know value placeholders ($X parameter) and SQL Names too, but you are a bit confused.

In your query you only assign values to placeholders. The database driver will handle them for you, providing proper escaping and variable substitution.

The documentation says:

When a parameter's data type is not specified or is declared as unknown, the type is inferred from the context in which the parameter is used (if possible).

I can't find a source that states what is the default type, but I think the INSERT statement provides enough context to identify the real types.

On the other hand you have to use SQL Names when you build your query dinamically. For example you have variable column or table names. They must be inserted through $1~ or $1:name style parameters keeping you safe from injection attacks.

Avoid SQL injection while inserting a mix of hard-coded and variable values?

1 Answers1