Skip to content

Commit 976364f

Browse files
nicolaswillnicolaswill
authored
Merge branch 'main' into operation_step_refactor
2 parents 0aee4f7 + 6038396 commit 976364f

File tree

539 files changed

+10151
-742
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

539 files changed

+10151
-742
lines changed

‎cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,49 @@ private predicate summaryModel0(
229229
)
230230
}
231231

232+
/**
233+
* Holds if the given extension tuple `madId` should pretty-print as `model`.
234+
*
235+
* This predicate should only be used in tests.
236+
*/
237+
predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
238+
exists(
239+
string namespace, string type, boolean subtypes, string name, string signature, string ext,
240+
string output, string kind, string provenance
241+
|
242+
Extensions::sourceModel(namespace, type, subtypes, name, signature, ext, output, kind,
243+
provenance, madId)
244+
|
245+
model =
246+
"Source: " + namespace + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; "
247+
+ ext + "; " + output + "; " + kind + "; " + provenance
248+
)
249+
or
250+
exists(
251+
string namespace, string type, boolean subtypes, string name, string signature, string ext,
252+
string input, string kind, string provenance
253+
|
254+
Extensions::sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance,
255+
madId)
256+
|
257+
model =
258+
"Sink: " + namespace + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
259+
ext + "; " + input + "; " + kind + "; " + provenance
260+
)
261+
or
262+
exists(
263+
string namespace, string type, boolean subtypes, string name, string signature, string ext,
264+
string input, string output, string kind, string provenance
265+
|
266+
Extensions::summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind,
267+
provenance, madId)
268+
|
269+
model =
270+
"Summary: " + namespace + "; " + type + "; " + subtypes + "; " + name + "; " + signature +
271+
"; " + ext + "; " + input + "; " + output + "; " + kind + "; " + provenance
272+
)
273+
}
274+
232275
/**
233276
* Holds if `input` is `input0`, but with all occurrences of `@` replaced
234277
* by `n` repetitions of `*` (and similarly for `output` and `output0`).

‎cpp/ql/lib/utils/test/PrettyPrintModels.ql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/**
2+
* @kind test-postprocess
3+
*/
4+
5+
import semmle.code.cpp.dataflow.ExternalFlow
6+
import codeql.dataflow.test.ProvenancePathGraph::TestPostProcessing::TranslateProvenanceResults<interpretModelForTest/2>

‎cpp/ql/test/library-tests/dataflow/external-models/flow.expected

Lines changed: 61 additions & 37 deletions
Large diffs are not rendered by default.

‎cpp/ql/test/library-tests/dataflow/external-models/flow.ql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import utils.test.dataflow.FlowTestCommon
22
import cpp
33
import semmle.code.cpp.security.FlowSources
4-
import IRTest::IRFlow::PathGraph
4+
import codeql.dataflow.test.ProvenancePathGraph
55

66
module IRTest {
77
private import semmle.code.cpp.ir.IR
@@ -33,3 +33,4 @@ module IRTest {
3333
}
3434

3535
import MakeTest<IRFlowTest<IRTest::IRFlow>>
36+
import ShowProvenance<interpretModelForTest/2, IRTest::IRFlow::PathNode, IRTest::IRFlow::PathGraph>

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
#select
2+
| test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | **argv | test.c:21:18:21:23 | *query1 | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | **argv | user input (a command-line argument) |
3+
| test.c:51:18:51:23 | query1 | test.c:14:27:14:30 | **argv | test.c:51:18:51:23 | *query1 | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | **argv | user input (a command-line argument) |
4+
| test.c:76:17:76:25 | userInput | test.c:75:8:75:16 | gets output argument | test.c:76:17:76:25 | *userInput | This argument to a SQL query function is derived from $@ and then passed to SQLPrepare(StatementText). | test.c:75:8:75:16 | gets output argument | user input (string read by gets) |
5+
| test.c:77:20:77:28 | userInput | test.c:75:8:75:16 | gets output argument | test.c:77:20:77:28 | *userInput | This argument to a SQL query function is derived from $@ and then passed to SQLExecDirect(StatementText). | test.c:75:8:75:16 | gets output argument | user input (string read by gets) |
6+
| 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) |
7+
| 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) |
8+
| 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) |
19
edges
210
| test.c:14:27:14:30 | **argv | test.c:15:20:15:26 | *access to array | provenance | |
311
| test.c:15:20:15:26 | *access to array | test.c:21:18:21:23 | *query1 | provenance | TaintFunction |
@@ -9,9 +17,12 @@ edges
917
| test.c:48:20:48:33 | *globalUsername | test.c:51:18:51:23 | *query1 | provenance | TaintFunction |
1018
| test.c:75:8:75:16 | gets output argument | test.c:76:17:76:25 | *userInput | provenance | |
1119
| test.c:75:8:75:16 | gets output argument | test.c:77:20:77:28 | *userInput | provenance | |
12-
| test.c:101:8:101:16 | gets output argument | test.c:106:24:106:29 | *query1 | provenance | TaintFunction Sink:MaD:325 |
13-
| test.c:101:8:101:16 | gets output argument | test.c:107:28:107:33 | *query1 | provenance | TaintFunction Sink:MaD:326 |
20+
| test.c:101:8:101:16 | gets output argument | test.c:106:24:106:29 | *query1 | provenance | TaintFunction Sink:MaD:2 |
21+
| test.c:101:8:101:16 | gets output argument | test.c:107:28:107:33 | *query1 | provenance | TaintFunction Sink:MaD:1 |
1422
| test.cpp:39:27:39:30 | **argv | test.cpp:43:27:43:33 | *access to array | provenance | |
23+
models
24+
| 1 | Sink: ; ; false; OCIStmtPrepare2; ; ; Argument[*3]; sql-injection; manual |
25+
| 2 | Sink: ; ; false; OCIStmtPrepare; ; ; Argument[*2]; sql-injection; manual |
1526
nodes
1627
| test.c:14:27:14:30 | **argv | semmle.label | **argv |
1728
| test.c:15:20:15:26 | *access to array | semmle.label | *access to array |
@@ -31,11 +42,3 @@ nodes
3142
| test.cpp:39:27:39:30 | **argv | semmle.label | **argv |
3243
| test.cpp:43:27:43:33 | *access to array | semmle.label | *access to array |
3344
subpaths
34-
#select
35-
| test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | **argv | test.c:21:18:21:23 | *query1 | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | **argv | user input (a command-line argument) |
36-
| test.c:51:18:51:23 | query1 | test.c:14:27:14:30 | **argv | test.c:51:18:51:23 | *query1 | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | **argv | user input (a command-line argument) |
37-
| test.c:76:17:76:25 | userInput | test.c:75:8:75:16 | gets output argument | test.c:76:17:76:25 | *userInput | This argument to a SQL query function is derived from $@ and then passed to SQLPrepare(StatementText). | test.c:75:8:75:16 | gets output argument | user input (string read by gets) |
38-
| test.c:77:20:77:28 | userInput | test.c:75:8:75:16 | gets output argument | test.c:77:20:77:28 | *userInput | This argument to a SQL query function is derived from $@ and then passed to SQLExecDirect(StatementText). | test.c:75:8:75:16 | gets output argument | user input (string read by gets) |
39-
| 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) |
40-
| 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) |
41-
| 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) |

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,5 @@
1-
Security/CWE/CWE-089/SqlTainted.ql
1+
query: Security/CWE/CWE-089/SqlTainted.ql
2+
postprocess:
3+
- utils/test/PrettyPrintModels.ql
4+
- utils/test/InlineExpectationsTestQuery.ql
5+

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ int atoi(const char *nptr);
1111
void exit(int i);
1212
///// Test code /////
1313

14-
int main(int argc, char** argv) {
14+
int main(int argc, char** argv) { // $ Source
1515
char *userName = argv[2];
1616
int userNumber = atoi(argv[3]);
1717

1818
// a string from the user is injected directly into an SQL query.
1919
char query1[1000] = {0};
2020
snprintf(query1, 1000, "SELECT UID FROM USERS where name = \"%s\"", userName);
21-
mysql_query(0, query1); // BAD
21+
mysql_query(0, query1); // $ Alert
2222

2323
// the user string is encoded by a library routine.
2424
char userNameSanitized[1000] = {0};
@@ -48,7 +48,7 @@ void badFunc() {
4848
char *userName = globalUsername;
4949
char query1[1000] = {0};
5050
snprintf(query1, 1000, "SELECT UID FROM USERS where name = \"%s\"", userName);
51-
mysql_query(0, query1); // BAD
51+
mysql_query(0, query1); // $ Alert
5252
}
5353

5454
//ODBC Library Rountines
@@ -72,9 +72,9 @@ SQLRETURN SQLPrepare(
7272

7373
void ODBCTests(){
7474
char userInput[100];
75-
gets(userInput);
76-
SQLPrepare(0, userInput, 100); // BAD
77-
SQLExecDirect(0, userInput, 100); // BAD
75+
gets(userInput); // $ Source
76+
SQLPrepare(0, userInput, 100); // $ Alert
77+
SQLExecDirect(0, userInput, 100); // $ Alert
7878
}
7979

8080
// Oracle Call Interface (OCI) Routines
@@ -98,13 +98,13 @@ int OCIStmtPrepare2(
9898

9999
void OCITests(){
100100
char userInput[100];
101-
gets(userInput);
101+
gets(userInput); // $ Source
102102

103103
// a string from the user is injected directly into an SQL query.
104104
char query1[1000] = {0};
105105
snprintf(query1, 1000, "SELECT UID FROM USERS where name = \"%s\"", userInput);
106-
OCIStmtPrepare(0, 0, query1, 0, 0, 0); // BAD
107-
OCIStmtPrepare2(0, 0, 0, query1, 0, 0, 0, 0, 0); // BAD
106+
OCIStmtPrepare(0, 0, query1, 0, 0, 0); // $ Alert
107+
OCIStmtPrepare2(0, 0, 0, query1, 0, 0, 0, 0, 0); // $ Alert
108108

109109
// an integer from the user is injected into an SQL query.
110110
int userNumber = atoi(userInput);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ namespace pqxx {
3636
};
3737
}
3838

39-
int main(int argc, char** argv) {
39+
int main(int argc, char** argv) { // $ Source
4040
pqxx::connection c;
4141
pqxx::work w(c);
4242

43-
pqxx::row r = w.exec1(argv[1]); // BAD
43+
pqxx::row r = w.exec1(argv[1]); // $ Alert
4444

4545
pqxx::result r2 = w.exec(w.quote(argv[1])); // GOOD
4646

‎csharp/ql/lib/ext/Microsoft.Data.SqlClient.model.yml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/csharp-all
4+
extensible: sinkModel
5+
data:
6+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String)", "", "Argument[0]", "sql-injection", "manual"]
7+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String,Microsoft.Data.SqlClient.SqlConnection)", "", "Argument[0]", "sql-injection", "manual"]
8+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String,Microsoft.Data.SqlClient.SqlConnection,Microsoft.Data.SqlClient.SqlTransaction)", "", "Argument[0]", "sql-injection", "manual"]
9+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String,Microsoft.Data.SqlClient.SqlConnection,Microsoft.Data.SqlClient.SqlTransaction,Microsoft.Data.SqlClient.SqlCommandColumnEncryptionSetting)", "", "Argument[0]", "sql-injection", "manual"]
10+
- ["Microsoft.Data.SqlClient", "SqlDataAdapter", False, "SqlDataAdapter", "(Microsoft.Data.SqlClient.SqlCommand)", "", "Argument[0]", "sql-injection", "manual"]
11+
- ["Microsoft.Data.SqlClient", "SqlDataAdapter", False, "SqlDataAdapter", "(System.String,Microsoft.Data.SqlClient.SqlConnection)", "", "Argument[0]", "sql-injection", "manual"]
12+
- ["Microsoft.Data.SqlClient", "SqlDataAdapter", False, "SqlDataAdapter", "(System.String,System.String)", "", "Argument[0]", "sql-injection", "manual"]
13+
- addsTo:
14+
pack: codeql/csharp-all
15+
extensible: summaryModel
16+
data:
17+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
18+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String,Microsoft.Data.SqlClient.SqlConnection)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
19+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String,Microsoft.Data.SqlClient.SqlConnection,Microsoft.Data.SqlClient.SqlTransaction)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
20+
- ["Microsoft.Data.SqlClient", "SqlCommand", False, "SqlCommand", "(System.String,Microsoft.Data.SqlClient.SqlConnection,Microsoft.Data.SqlClient.SqlTransaction,Microsoft.Data.SqlClient.SqlCommandColumnEncryptionSetting)", "", "Argument[0]", "Argument[this]", "taint", "manual"]

‎csharp/ql/lib/semmle/code/csharp/dataflow/Bound.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
/**
22
* Provides classes for representing abstract bounds for use in, for example, range analysis.
33
*/
4+
overlay[local?]
5+
module;
46

57
private import internal.rangeanalysis.BoundSpecific
68

‎csharp/ql/lib/semmle/code/csharp/dataflow/ModulusAnalysis.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* an expression, `b` is a `Bound` (typically zero or the value of an SSA
44
* variable), and `v` is an integer in the range `[0 .. m-1]`.
55
*/
6+
overlay[local?]
7+
module;
68

79
private import internal.rangeanalysis.ModulusAnalysisSpecific::Private
810
private import Bound

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/Sign.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
overlay[local?]
2+
module;
3+
14
newtype TSign =
25
TNeg() or
36
TZero() or

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* The analysis is implemented as an abstract interpretation over the
66
* three-valued domain `{negative, zero, positive}`.
77
*/
8+
overlay[local?]
9+
module;
810

911
private import SignAnalysisSpecific::Private
1012
private import SsaReadPositionCommon

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaReadPositionCommon.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
/**
22
* Provides classes for representing a position at which an SSA variable is read.
33
*/
4+
overlay[local?]
5+
module;
46

57
private import SsaReadPositionSpecific
68
import SsaReadPositionSpecific::Public

‎csharp/ql/lib/semmle/code/csharp/frameworks/Sql.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class IDbCommandConstructionSqlExpr extends SqlExpr, ObjectCreation {
3535
ic.getParameter(0).getType() instanceof StringType and
3636
not exists(Type t | t = ic.getDeclaringType() |
3737
// Known sealed classes:
38+
t.hasFullyQualifiedName("Microsoft.Data.SqlClient", "SqlCommand") or
3839
t.hasFullyQualifiedName("System.Data.SqlClient", "SqlCommand") or
3940
t.hasFullyQualifiedName("System.Data.Odbc", "OdbcCommand") or
4041
t.hasFullyQualifiedName("System.Data.OleDb", "OleDbCommand") or

‎csharp/ql/src/change-notes/2025-06-25-sqlcommand-models.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 explicit SQL injection Models as Data models for `Microsoft.Data.SqlClient.SqlCommand` and `Microsoft.Data.SqlClient.SqlDataAdapter`. This reduces false negatives for the query `cs/sql-injection`.

‎csharp/ql/test/query-tests/Security Features/CWE-089-2/SqlInjection.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
using System;
2+
using Microsoft.Data;
3+
using Microsoft.Data.SqlClient;
4+
5+
namespace Test
6+
{
7+
class SqlInjection
8+
{
9+
string connectionString;
10+
System.Windows.Forms.TextBox box1;
11+
12+
public void MakeSqlCommand()
13+
{
14+
// BAD: Text from a local textbox
15+
using (var connection = new SqlConnection(connectionString))
16+
{
17+
var queryString = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
18+
+ box1.Text + "' ORDER BY PRICE"; // $ Source[cs/sql-injection]
19+
var cmd = new SqlCommand(queryString); // $ Alert[cs/sql-injection]
20+
var adapter = new SqlDataAdapter(cmd); // $ Alert[cs/sql-injection]
21+
}
22+
23+
// BAD: Input from the command line.
24+
using (var connection = new SqlConnection(connectionString))
25+
{
26+
var queryString = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
27+
+ Console.ReadLine() + "' ORDER BY PRICE"; // $ Source[cs/sql-injection]
28+
var cmd = new SqlCommand(queryString); // $ Alert[cs/sql-injection]
29+
var adapter = new SqlDataAdapter(cmd); // $ Alert[cs/sql-injection]
30+
}
31+
}
32+
}
33+
}

0 commit comments

Comments
 (0)