How can I replace this obviously bad string interpolation with a prepared statement?

Question

Apart from checking that the attributeId is in a list of predefined strings – possible, but in this particular case costly – is there any way to re-write this in a less 'injectable' fashion (since the use of '${attributeId}' is actually a field, and not strictly a parameter...):

const { tablename, attributeId } = req.params;
  let stmt = `SELECT DISTINCT row_data->'${attributeId}' as ${attributeId} FROM csv_data WHERE source = '${tablename}'`;
  let result;

  try {
    await pool.connect();
    result = await pool.query(stmt);
  } catch (err) {
    console.error(err);
  }

  ...

  return res.status(200).json({
    rows: result.rows.map((elt, i, array) => {
      return elt[attributeId];
    }),
    rowCount: result.rowCount,
  });

First thing in the documentation https://node-postgres.com/#getting-started — Konrad, Apr 23 '23 at 16:04
Really? JSONB subqueries are the first thing in the documentation? Who knew? — Dycey, Apr 23 '23 at 16:35
You don't want a prepared statement, you want a [*parameterised* statement](https://node-postgres.com/features/queries#parameterized-query). — Bergi, Apr 23 '23 at 17:05
…and don't use `as ${attributeId}` but rather a fixed column name — Bergi, Apr 23 '23 at 17:05

score 0 · Answer 1 · answered Apr 23 '23 at 22:50

You don't want a prepared statement, you want a parameterised statement:

const { tablename, attributeId } = req.params;
const stmt = 'SELECT DISTINCT row_data->$1 as attribute FROM csv_data WHERE source = $2;';
//                                      ^^                                           ^^
const result = await pool.query(stmt, [attributeId, tableName]);
//                                    ^^^^^^^^^^^^^^^^^^^^^^^^

res.status(200).json({
  rows: result.rows.map(elt => elt.attribute),
  rowCount: result.rowCount,
});

Also

don't use a dynamic column name, if you really needed to, you'd have to escape it
don't call pool.connect() if you want to use a one-shot query instead of a client (that you'd later have to release)
don't use try/catch only around the query, and don't console.error instead of sending a proper error response

Your approach doesn't want to work with JSONB column selection sady. — Dycey, Apr 24 '23 at 21:28

score 0 · Answer 2 · answered Apr 24 '23 at 21:29

;-) Actually I do want to use dynamic column names, for a very specific purpose, and strangely that wasn't production code so console.error() is fine. But thanks for your concern.

The solution turns out to be pg-format:

const format = require("pg-format");

const stmt = format(
  "SELECT DISTINCT row_data->%L as %I FROM csv_data WHERE source = %L",
  attributeId,
  attributeId,
  tablename
);

Gives me back exactly what I was looking for, as well as handling the escaping:

"SELECT DISTINCT row_data->'state_code' as state_code FROM csv_data WHERE source = 'EQP'"

I definitely don't need the await pool.connect(); though ;-)

How can I replace this obviously bad string interpolation with a prepared statement?

2 Answers2