Skip to content

cpp: model libpq query functions as SQL-injection sinks - #22449

Open
kumarak wants to merge 2 commits into
github:mainfrom
trail-of-forks:kumarak/cpp-libpq-sql-sinks
Open

cpp: model libpq query functions as SQL-injection sinks#22449
kumarak wants to merge 2 commits into
github:mainfrom
trail-of-forks:kumarak/cpp-libpq-sql-sinks

Conversation

@kumarak

@kumarak kumarak commented Aug 27, 2026

Copy link
Copy Markdown

This PR adds SQL injection detection support for C/C++ code using the PostgreSQL libpq API. It models the SQL query arguments of PQexec, PQexecParams, PQprepare, PQsendQuery, PQsendQueryParams, and PQsendPrepare as sinks for the Uncontrolled data used in SQL queries.

It also adds tests covering both tainted and safe inputs for each modeled API, updates the expected results, and includes a change note documenting the new libpq modeling.

Add the PostgreSQL libpq query-execution functions as sql-injection
sinks so cpp/sql-injection flags tainted data reaching their SQL text:

- PQexec / PQsendQuery: query at argument 1
- PQexecParams / PQsendQueryParams: command at argument 1
- PQprepare / PQsendPrepare: query at argument 2

The *Prepared forms take a prepared-statement name rather than SQL text
and are not modeled. Follows the existing Oracle.oci sink convention.
Copilot AI balanced review requested due to automatic review settings August 27, 2026 15:51
@kumarak
kumarak requested a review from a team as a code owner August 27, 2026 15:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds libpq SQL-injection sink modeling for C/C++ PostgreSQL applications.

Changes:

  • Models six synchronous and asynchronous libpq query APIs.
  • Adds positive tests and a change note.
  • Updates expected query results.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
cpp/ql/lib/ext/Postgres.model.yml Defines libpq SQL-injection sinks.
cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/test_libpq.c Exercises the new models.
cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected Records expected alerts and models.
cpp/ql/lib/change-notes/2026-08-27-libpq-sql-sinks.md Documents the added analysis support.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments. In general this looks good.

---
category: minorAnalysis
---
* Added the PostgreSQL libpq query-execution functions (`PQexec`, `PQexecParams`, `PQprepare`, and their asynchronous `PQsendQuery`/`PQsendQueryParams`/`PQsendPrepare` counterparts) as `sql-injection` sinks, so the "Uncontrolled data used in SQL query" query flags tainted data reaching their command/query arguments.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll just keep this short:

Suggested change
* Added the PostgreSQL libpq query-execution functions (`PQexec`, `PQexecParams`, `PQprepare`, and their asynchronous `PQsendQuery`/`PQsendQueryParams`/`PQsendPrepare` counterparts) as `sql-injection` sinks, so the "Uncontrolled data used in SQL query" query flags tainted data reaching their command/query arguments.
* Added the PostgreSQL libpq (asynchronous) query-execution functions `PQexec`, `PQexecParams`, `PQprepare`, `PQsendQuery`, `PQsendQueryParams`, `PQsendPrepare` as `sql-injection` sinks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated based on suggestions.

Comment on lines +1 to +3
// Test case for the PostgreSQL libpq SQL-injection sinks modeled in Postgres.model.yml.
// Associated with CWE-089: SQL injection.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obvious from the name of the test and the location.

Suggested change
// Test case for the PostgreSQL libpq SQL-injection sinks modeled in Postgres.model.yml.
// Associated with CWE-089: SQL injection.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 4796ff4

Comment thread cpp/ql/lib/ext/Postgres.model.yml Outdated
Comment on lines +7 to +10
# Functions that take a raw SQL command string. The `*Params`/`*Prepare` forms still
# interpret their command/query text as SQL, so injection into that text is a sink even
# though the parameter *values* are passed separately. The `*Prepared` forms take a
# prepared-statement name rather than SQL text and are therefore not modeled here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the nature of this file, this seems all pretty obvious and not adding much. I would shorten this to:

Suggested change
# Functions that take a raw SQL command string. The `*Params`/`*Prepare` forms still
# interpret their command/query text as SQL, so injection into that text is a sink even
# though the parameter *values* are passed separately. The `*Prepared` forms take a
# prepared-statement name rather than SQL text and are therefore not modeled here.
# The `*Params`/`*Prepare` functions interpret their command/query text as SQL,
# so injection into that text is a sink even though the parameter values are
# passed separately. The `*Prepared` functions take a prepared-statement name
# rather than SQL text and are therefore not modeled here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

Trim the model-file comment and change note to house style, drop the
redundant test header comment, and regenerate the expected results for
the shifted line numbers.
@kumarak
kumarak requested a review from jketema August 30, 2026 20:22
@jketema

jketema commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Looks like your changes broke cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.qlref (this is due to that test being fragile when new models get added).

Best way to fix this:

  1. update NonConstantFormat.qlref to:
    query: Likely Bugs/Format/NonConstantFormat.ql
    postprocess:
     - utils/test/PrettyPrintModels.ql
     - utils/test/InlineExpectationsTestQuery.ql
    
  2. accept the test result changes

The first of these will make the test less fragile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants