Skip to content

Commit 4048b13

Browse files
geoffw0geoffw0
authored
Merge pull request #22562 from trail-of-forks/comdb2-sql-injection-models
C++: Model Comdb2 SQL execution sinks
2 parents e387ca4 + 75d9457 commit 4048b13

4 files changed

Lines changed: 56 additions & 0 deletions

File tree

‎cpp/ql/lib/change-notes/2026-09-14-comdb2-sql-injection.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added SQL-injection sink models for the Comdb2 C API functions `cdb2_run_statement` and `cdb2_run_statement_typed`.

‎cpp/ql/lib/ext/Comdb2.model.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Partial model of the Comdb2 C API.
2+
# https://bloomberg.github.io/comdb2/c_api.html
3+
extensions:
4+
- addsTo:
5+
pack: codeql/cpp-all
6+
extensible: sinkModel
7+
data: # namespace, type, subtypes, name, signature, ext, input, kind, provenance
8+
# Both functions interpret the second argument as SQL. The typed variant
9+
# specifies result-column types; bound parameter values are passed separately.
10+
- ["", "", False, "cdb2_run_statement", "", "", "Argument[*1]", "sql-injection", "manual"]
11+
- ["", "", False, "cdb2_run_statement_typed", "", "", "Argument[*1]", "sql-injection", "manual"]

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
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_comdb2.c:22:28:22:36 | userInput | test_comdb2.c:18:8:18:16 | gets output argument | test_comdb2.c:22:28:22:36 | userInput | This argument to a SQL query function is derived from $@. | test_comdb2.c:18:8:18:16 | gets output argument | user input (string read by gets) |
10+
| test_comdb2.c:23:34:23:42 | userInput | test_comdb2.c:18:8:18:16 | gets output argument | test_comdb2.c:23:34:23:42 | userInput | This argument to a SQL query function is derived from $@. | test_comdb2.c:18:8:18:16 | gets output argument | user input (string read by gets) |
911
| 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) |
1012
| 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) |
1113
| 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) |
@@ -26,6 +28,8 @@ edges
2628
| test.c:101:8:101:16 | gets output argument | test.c:106:24:106:29 | query1 | provenance | TaintFunction Sink:MaD:2 |
2729
| test.c:101:8:101:16 | gets output argument | test.c:107:28:107:33 | query1 | provenance | TaintFunction Sink:MaD:1 |
2830
| test.cpp:39:27:39:30 | **argv | test.cpp:43:27:43:33 | *access to array | provenance | |
31+
| test_comdb2.c:18:8:18:16 | gets output argument | test_comdb2.c:22:28:22:36 | userInput | provenance | Sink:MaD:9 |
32+
| test_comdb2.c:18:8:18:16 | gets output argument | test_comdb2.c:23:34:23:42 | userInput | provenance | Sink:MaD:10 |
2933
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:26:16:26:24 | userInput | provenance | Sink:MaD:3 |
3034
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:27:22:27:30 | userInput | provenance | Sink:MaD:4 |
3135
| test_libpq.c:23:8:23:16 | gets output argument | test_libpq.c:28:27:28:35 | userInput | provenance | Sink:MaD:5 |
@@ -41,6 +45,8 @@ models
4145
| 6 | Sink: ; ; false; PQsendPrepare; ; ; Argument[*2]; sql-injection; manual |
4246
| 7 | Sink: ; ; false; PQsendQuery; ; ; Argument[*1]; sql-injection; manual |
4347
| 8 | Sink: ; ; false; PQsendQueryParams; ; ; Argument[*1]; sql-injection; manual |
48+
| 9 | Sink: ; ; false; cdb2_run_statement; ; ; Argument[*1]; sql-injection; manual |
49+
| 10 | Sink: ; ; false; cdb2_run_statement_typed; ; ; Argument[*1]; sql-injection; manual |
4450
nodes
4551
| test.c:14:27:14:30 | **argv | semmle.label | **argv |
4652
| test.c:15:20:15:26 | *access to array | semmle.label | *access to array |
@@ -59,6 +65,9 @@ nodes
5965
| test.c:107:28:107:33 | query1 | semmle.label | query1 |
6066
| test.cpp:39:27:39:30 | **argv | semmle.label | **argv |
6167
| test.cpp:43:27:43:33 | *access to array | semmle.label | *access to array |
68+
| test_comdb2.c:18:8:18:16 | gets output argument | semmle.label | gets output argument |
69+
| test_comdb2.c:22:28:22:36 | userInput | semmle.label | userInput |
70+
| test_comdb2.c:23:34:23:42 | userInput | semmle.label | userInput |
6271
| test_libpq.c:23:8:23:16 | gets output argument | semmle.label | gets output argument |
6372
| test_libpq.c:26:16:26:24 | userInput | semmle.label | userInput |
6473
| test_libpq.c:27:22:27:30 | userInput | semmle.label | userInput |

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Minimal declarations from the public Comdb2 C API:
2+
// https://github.com/bloomberg/comdb2/blob/dfb15415b48cbe67220d76964243086051813676/cdb2api/cdb2api.h
3+
typedef struct cdb2_hndl cdb2_hndl_tp;
4+
5+
enum { CDB2_CSTRING = 3 };
6+
7+
int cdb2_run_statement(cdb2_hndl_tp *hndl, const char *sql);
8+
int cdb2_run_statement_typed(cdb2_hndl_tp *hndl, const char *sql, int ntypes, const int *types);
9+
int cdb2_bind_param(cdb2_hndl_tp *hndl, const char *name, int type,
10+
const void *varaddr, int length);
11+
12+
char *gets(char *s);
13+
typedef unsigned long size_t;
14+
size_t strlen(const char *s);
15+
16+
void comdb2Tests(cdb2_hndl_tp *hndl) {
17+
char userInput[1000];
18+
gets(userInput); // $ Source
19+
int types[] = {CDB2_CSTRING};
20+
21+
// Both execution functions interpret user-controlled query text as SQL.
22+
cdb2_run_statement(hndl, userInput); // $ Alert
23+
cdb2_run_statement_typed(hndl, userInput, 1, types); // $ Alert
24+
25+
cdb2_run_statement(hndl, "SELECT 1"); // GOOD
26+
cdb2_run_statement_typed(hndl, "SELECT 'constant'", 1, types); // GOOD
27+
28+
// Bound values are data, not SQL text, even when controlled by the user.
29+
cdb2_bind_param(hndl, "value", CDB2_CSTRING, userInput, strlen(userInput) + 1); // GOOD
30+
cdb2_run_statement(hndl, "SELECT @value"); // GOOD
31+
cdb2_run_statement_typed(hndl, "SELECT @value", 1, types); // GOOD
32+
}

0 commit comments

Comments
 (0)