Skip to content

Commit 4796ff4

Browse files
committed
cpp: address review feedback on libpq SQL-injection sinks
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.
1 parent 4623819 commit 4796ff4

4 files changed

Lines changed: 24 additions & 27 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* 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.
4+
* Added the PostgreSQL libpq (asynchronous) query-execution functions `PQexec`, `PQexecParams`, `PQprepare`, `PQsendQuery`, `PQsendQueryParams`, `PQsendPrepare` as `sql-injection` sinks.

cpp/ql/lib/ext/Postgres.model.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ extensions:
44
pack: codeql/cpp-all
55
extensible: sinkModel
66
data: # namespace, type, subtypes, name, signature, ext, input, kind, provenance
7-
# Functions that take a raw SQL command string. The `*Params`/`*Prepare` forms still
8-
# interpret their command/query text as SQL, so injection into that text is a sink even
9-
# though the parameter *values* are passed separately. The `*Prepared` forms take a
10-
# prepared-statement name rather than SQL text and are therefore not modeled here.
7+
# The `*Params`/`*Prepare` functions interpret their command/query text as SQL,
8+
# so injection into that text is a sink even though the parameter values are
9+
# passed separately. The `*Prepared` functions take a prepared-statement name
10+
# rather than SQL text and are therefore not modeled here.
1111
- ["", "", False, "PQexec", "", "", "Argument[*1]", "sql-injection", "manual"]
1212
- ["", "", False, "PQexecParams", "", "", "Argument[*1]", "sql-injection", "manual"]
1313
- ["", "", False, "PQprepare", "", "", "Argument[*2]", "sql-injection", "manual"]

cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
| test.c:106:24:106:29 | query1 | test.c:101:8:101:16 | gets output argument | test.c:106:24:106:29 | query1 | This argument to a SQL query function is derived from $@. | test.c:101:8:101:16 | gets output argument | user input (string read by gets) |
77
| test.c:107:28:107:33 | query1 | test.c:101:8:101:16 | gets output argument | test.c:107:28:107:33 | query1 | This argument to a SQL query function is derived from $@. | test.c:101:8:101:16 | gets output argument | user input (string read by gets) |
88
| test.cpp:43:27:43:33 | *access to array | test.cpp:39:27:39:30 | **argv | test.cpp:43:27:43:33 | *access to array | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | **argv | user input (a command-line argument) |
9-
| test_libpq.c:29:16:29:24 | userInput | test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:29:16:29:24 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:26:8:26:16 | gets output argument | user input (string read by gets) |
10-
| test_libpq.c:30:22:30:30 | userInput | test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:30:22:30:30 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:26:8:26:16 | gets output argument | user input (string read by gets) |
11-
| test_libpq.c:31:27:31:35 | userInput | test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:31:27:31:35 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:26:8:26:16 | gets output argument | user input (string read by gets) |
12-
| test_libpq.c:32:21:32:29 | userInput | test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:32:21:32:29 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:26:8:26:16 | gets output argument | user input (string read by gets) |
13-
| test_libpq.c:33:27:33:35 | userInput | test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:33:27:33:35 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:26:8:26:16 | gets output argument | user input (string read by gets) |
14-
| test_libpq.c:34:31:34:39 | userInput | test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:34:31:34:39 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:26:8:26:16 | gets output argument | user input (string read by gets) |
9+
| test_libpq.c:26:16:26:24 | userInput | test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:26:16:26:24 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:23:8:23:16 | gets output argument | user input (string read by gets) |
10+
| test_libpq.c:27:22:27:30 | userInput | test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:27:22:27:30 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:23:8:23:16 | gets output argument | user input (string read by gets) |
11+
| test_libpq.c:28:27:28:35 | userInput | test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:28:27:28:35 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:23:8:23:16 | gets output argument | user input (string read by gets) |
12+
| test_libpq.c:29:21:29:29 | userInput | test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:29:21:29:29 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:23:8:23:16 | gets output argument | user input (string read by gets) |
13+
| test_libpq.c:30:27:30:35 | userInput | test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:30:27:30:35 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:23:8:23:16 | gets output argument | user input (string read by gets) |
14+
| test_libpq.c:31:31:31:39 | userInput | test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:31:31:31:39 | userInput | This argument to a SQL query function is derived from $@. | test_libpq.c:23:8:23:16 | gets output argument | user input (string read by gets) |
1515
edges
1616
| test.c:14:27:14:30 | **argv | test.c:15:20:15:26 | *access to array | provenance | |
1717
| test.c:15:20:15:26 | *access to array | test.c:21:18:21:23 | *query1 | provenance | TaintFunction |
@@ -26,12 +26,12 @@ edges
2626
| test.c:101:8:101:16 | gets output argument | test.c:106:24:106:29 | query1 | provenance | TaintFunction Sink:MaD:2 |
2727
| test.c:101:8:101:16 | gets output argument | test.c:107:28:107:33 | query1 | provenance | TaintFunction Sink:MaD:1 |
2828
| test.cpp:39:27:39:30 | **argv | test.cpp:43:27:43:33 | *access to array | provenance | |
29-
| test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:29:16:29:24 | userInput | provenance | Sink:MaD:3 |
30-
| test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:30:22:30:30 | userInput | provenance | Sink:MaD:4 |
31-
| test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:31:27:31:35 | userInput | provenance | Sink:MaD:5 |
32-
| test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:32:21:32:29 | userInput | provenance | Sink:MaD:7 |
33-
| test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:33:27:33:35 | userInput | provenance | Sink:MaD:8 |
34-
| test_libpq.c:26:8:26:16 | gets output argument | test_libpq.c:34:31:34:39 | userInput | provenance | Sink:MaD:6 |
29+
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:26:16:26:24 | userInput | provenance | Sink:MaD:3 |
30+
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:27:22:27:30 | userInput | provenance | Sink:MaD:4 |
31+
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:28:27:28:35 | userInput | provenance | Sink:MaD:5 |
32+
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:29:21:29:29 | userInput | provenance | Sink:MaD:7 |
33+
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:30:27:30:35 | userInput | provenance | Sink:MaD:8 |
34+
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:31:31:31:39 | userInput | provenance | Sink:MaD:6 |
3535
models
3636
| 1 | Sink: ; ; false; OCIStmtPrepare2; ; ; Argument[*3]; sql-injection; manual |
3737
| 2 | Sink: ; ; false; OCIStmtPrepare; ; ; Argument[*2]; sql-injection; manual |
@@ -59,11 +59,11 @@ nodes
5959
| test.c:107:28:107:33 | query1 | semmle.label | query1 |
6060
| test.cpp:39:27:39:30 | **argv | semmle.label | **argv |
6161
| test.cpp:43:27:43:33 | *access to array | semmle.label | *access to array |
62-
| test_libpq.c:26:8:26:16 | gets output argument | semmle.label | gets output argument |
63-
| test_libpq.c:29:16:29:24 | userInput | semmle.label | userInput |
64-
| test_libpq.c:30:22:30:30 | userInput | semmle.label | userInput |
65-
| test_libpq.c:31:27:31:35 | userInput | semmle.label | userInput |
66-
| test_libpq.c:32:21:32:29 | userInput | semmle.label | userInput |
67-
| test_libpq.c:33:27:33:35 | userInput | semmle.label | userInput |
68-
| test_libpq.c:34:31:34:39 | userInput | semmle.label | userInput |
62+
| test_libpq.c:23:8:23:16 | gets output argument | semmle.label | gets output argument |
63+
| test_libpq.c:26:16:26:24 | userInput | semmle.label | userInput |
64+
| test_libpq.c:27:22:27:30 | userInput | semmle.label | userInput |
65+
| test_libpq.c:28:27:28:35 | userInput | semmle.label | userInput |
66+
| test_libpq.c:29:21:29:29 | userInput | semmle.label | userInput |
67+
| test_libpq.c:30:27:30:35 | userInput | semmle.label | userInput |
68+
| test_libpq.c:31:31:31:39 | userInput | semmle.label | userInput |
6969
subpaths

cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/test_libpq.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
// Test case for the PostgreSQL libpq SQL-injection sinks modeled in Postgres.model.yml.
2-
// Associated with CWE-089: SQL injection.
3-
41
typedef unsigned long size_t;
52
typedef unsigned int Oid;
63
typedef struct pg_conn PGconn;

0 commit comments

Comments
 (0)