SQL Injection Vulnerability (ES2015 template strings)

[bergkvist]

Using ES2015 template strings might make the SQL query readable, but it is NOT SAFE!

The end user of the API has control over req.query and req.params, and can therefore execute any SQL query/procedure they want.

Make sure to use a parameterized query when using postgres-node (pg).

This will make sure that whatever the user passes in will always be interpreted as data, and never as a command.

Example from your code:

dirt-simple-postgis-http-api/routes/nearest.js

Lines 2 to 30 in ebb8313

	const sql = (params, query) => {
	const [x, y, srid] = params.point.match(/^((-?\d+\.?\d+)(,-?\d+\.?\d+)(,[0-9]{4}))/)[0].split(',')
	return `
	SELECT
	${query.columns},
	ST_Distance(
	ST_Transform(
	st_setsrid(st_makepoint(${x}, ${y}), ${srid}),
	(SELECT ST_SRID(${query.geom_column}) FROM ${params.table} LIMIT 1)
	),
	${query.geom_column}
	) as distance
	FROM
	${params.table}
	-- Optional Filter
	${query.filter ? `WHERE ${query.filter}` : '' }
	ORDER BY
	${query.geom_column} <-> ST_Transform(
	st_setsrid(st_makepoint(${x}, ${y}), ${srid}),
	(SELECT ST_SRID(${query.geom_column}) FROM ${params.table} LIMIT 1)
	)
	LIMIT ${query.limit}
	`
	}

[tobinbradley]

Thanks for the suggestion!

You've identified the problem correctly - SQL injection is possible here. Swagger does some basic input typing, but that won't catch everything. Unfortunately the solution you proposed won't work.

Column and table names are only allowed as literals in Postgres - they cannot be parameterized. I could only paramaterize part of the SQL string, which is as effective as paramaterizing none of it. Note this isn't a node-postgres thing, it's a Postgres thing.

I'm going to close this as the solution you brought forward won't work, but if you do have a different idea please let me know. I've hunted around for a reasonable SQL injection protection mechanism that's better than the input sanitization step Swagger uses, but I haven't found anything that wasn't string processing whack-a-mole.

Again, thanks for the suggestion!

[bergkvist]

You are correct in that node-postgres does not support parameterized table/column names.

What the contributor is saying about this not being a limitation of node-postgres is wrong. This is something the library could clearly have supported if it wanted to.

To prove my point, and give you a recommendation, check out pg-promise.

Parameterized table/column names in pg-promise are supported by simply adding ":name" or "~" after the parameter.