Skip to content

Commit 2f208bd

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #19877 from michaelnebel/csharp/microsoftdatasqlclient
C#: Models for Microsoft.Data.SqlClient.
2 parents f568d41 + cfadd30 commit 2f208bd

File tree

66 files changed

+7797
-42
lines changed

Some content is hidden

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

66 files changed

+7797
-42
lines changed

‎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/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+
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#select
2+
| SqlInjection.cs:19:42:19:52 | access to local variable queryString | SqlInjection.cs:18:21:18:29 | access to property Text : String | SqlInjection.cs:19:42:19:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:18:21:18:29 | access to property Text : String | this TextBox text |
3+
| SqlInjection.cs:20:50:20:52 | access to local variable cmd | SqlInjection.cs:18:21:18:29 | access to property Text : String | SqlInjection.cs:20:50:20:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:18:21:18:29 | access to property Text : String | this TextBox text |
4+
| SqlInjection.cs:28:42:28:52 | access to local variable queryString | SqlInjection.cs:27:21:27:38 | call to method ReadLine : String | SqlInjection.cs:28:42:28:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:27:21:27:38 | call to method ReadLine : String | this read from stdin |
5+
| SqlInjection.cs:29:50:29:52 | access to local variable cmd | SqlInjection.cs:27:21:27:38 | call to method ReadLine : String | SqlInjection.cs:29:50:29:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:27:21:27:38 | call to method ReadLine : String | this read from stdin |
6+
edges
7+
| SqlInjection.cs:17:21:17:31 | access to local variable queryString : String | SqlInjection.cs:19:42:19:52 | access to local variable queryString | provenance | Sink:MaD:1 |
8+
| SqlInjection.cs:17:21:17:31 | access to local variable queryString : String | SqlInjection.cs:19:42:19:52 | access to local variable queryString : String | provenance | |
9+
| SqlInjection.cs:18:21:18:29 | access to property Text : String | SqlInjection.cs:17:21:17:31 | access to local variable queryString : String | provenance | |
10+
| SqlInjection.cs:19:21:19:23 | access to local variable cmd : SqlCommand | SqlInjection.cs:20:50:20:52 | access to local variable cmd | provenance | Sink:MaD:2 |
11+
| SqlInjection.cs:19:27:19:53 | object creation of type SqlCommand : SqlCommand | SqlInjection.cs:19:21:19:23 | access to local variable cmd : SqlCommand | provenance | |
12+
| SqlInjection.cs:19:42:19:52 | access to local variable queryString : String | SqlInjection.cs:19:27:19:53 | object creation of type SqlCommand : SqlCommand | provenance | MaD:4 |
13+
| SqlInjection.cs:26:21:26:31 | access to local variable queryString : String | SqlInjection.cs:28:42:28:52 | access to local variable queryString | provenance | Sink:MaD:1 |
14+
| SqlInjection.cs:26:21:26:31 | access to local variable queryString : String | SqlInjection.cs:28:42:28:52 | access to local variable queryString : String | provenance | |
15+
| SqlInjection.cs:27:21:27:38 | call to method ReadLine : String | SqlInjection.cs:26:21:26:31 | access to local variable queryString : String | provenance | Src:MaD:3 |
16+
| SqlInjection.cs:28:21:28:23 | access to local variable cmd : SqlCommand | SqlInjection.cs:29:50:29:52 | access to local variable cmd | provenance | Sink:MaD:2 |
17+
| SqlInjection.cs:28:27:28:53 | object creation of type SqlCommand : SqlCommand | SqlInjection.cs:28:21:28:23 | access to local variable cmd : SqlCommand | provenance | |
18+
| SqlInjection.cs:28:42:28:52 | access to local variable queryString : String | SqlInjection.cs:28:27:28:53 | object creation of type SqlCommand : SqlCommand | provenance | MaD:4 |
19+
models
20+
| 1 | Sink: Microsoft.Data.SqlClient; SqlCommand; false; SqlCommand; (System.String); ; Argument[0]; sql-injection; manual |
21+
| 2 | Sink: Microsoft.Data.SqlClient; SqlDataAdapter; false; SqlDataAdapter; (Microsoft.Data.SqlClient.SqlCommand); ; Argument[0]; sql-injection; manual |
22+
| 3 | Source: System; Console; false; ReadLine; ; ; ReturnValue; stdin; manual |
23+
| 4 | Summary: Microsoft.Data.SqlClient; SqlCommand; false; SqlCommand; (System.String); ; Argument[0]; Argument[this]; taint; manual |
24+
nodes
25+
| SqlInjection.cs:17:21:17:31 | access to local variable queryString : String | semmle.label | access to local variable queryString : String |
26+
| SqlInjection.cs:18:21:18:29 | access to property Text : String | semmle.label | access to property Text : String |
27+
| SqlInjection.cs:19:21:19:23 | access to local variable cmd : SqlCommand | semmle.label | access to local variable cmd : SqlCommand |
28+
| SqlInjection.cs:19:27:19:53 | object creation of type SqlCommand : SqlCommand | semmle.label | object creation of type SqlCommand : SqlCommand |
29+
| SqlInjection.cs:19:42:19:52 | access to local variable queryString | semmle.label | access to local variable queryString |
30+
| SqlInjection.cs:19:42:19:52 | access to local variable queryString : String | semmle.label | access to local variable queryString : String |
31+
| SqlInjection.cs:20:50:20:52 | access to local variable cmd | semmle.label | access to local variable cmd |
32+
| SqlInjection.cs:26:21:26:31 | access to local variable queryString : String | semmle.label | access to local variable queryString : String |
33+
| SqlInjection.cs:27:21:27:38 | call to method ReadLine : String | semmle.label | call to method ReadLine : String |
34+
| SqlInjection.cs:28:21:28:23 | access to local variable cmd : SqlCommand | semmle.label | access to local variable cmd : SqlCommand |
35+
| SqlInjection.cs:28:27:28:53 | object creation of type SqlCommand : SqlCommand | semmle.label | object creation of type SqlCommand : SqlCommand |
36+
| SqlInjection.cs:28:42:28:52 | access to local variable queryString | semmle.label | access to local variable queryString |
37+
| SqlInjection.cs:28:42:28:52 | access to local variable queryString : String | semmle.label | access to local variable queryString : String |
38+
| SqlInjection.cs:29:50:29:52 | access to local variable cmd | semmle.label | access to local variable cmd |
39+
subpaths

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
3+
- addsTo:
4+
pack: codeql/threat-models
5+
extensible: threatModelConfiguration
6+
data:
7+
- ["local", true, 0]

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

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

‎csharp/ql/test/query-tests/Security Features/CWE-089-2/options

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
semmle-extractor-options: /nostdlib /noconfig
2+
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/Microsoft.Data.SqlClient/6.0.2/Microsoft.Data.SqlClient.csproj
3+
semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Windows.cs
4+
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,20 @@ public void ProcessRequest()
1717
{
1818
connection.Open();
1919
SqlCommand customerCommand = new SqlCommand("SELECT * FROM customers", connection);
20-
SqlDataReader customerReader = customerCommand.ExecuteReader();
20+
SqlDataReader customerReader = customerCommand.ExecuteReader(); // $ Source[cs/sql-injection]
2121

2222
while (customerReader.Read())
2323
{
2424
// BAD: Read from database, write it straight to another query
25-
SqlCommand secondCustomerCommand = new SqlCommand("SELECT * FROM customers WHERE customerName=" + customerReader.GetString(1), connection);
25+
SqlCommand secondCustomerCommand = new SqlCommand("SELECT * FROM customers WHERE customerName=" + customerReader.GetString(1), connection); // $ Alert[cs/sql-injection]
2626
}
2727
customerReader.Close();
2828
}
2929
}
3030

3131
public void RunSQLFromFile()
3232
{
33-
using (FileStream fs = new FileStream("myfile.txt", FileMode.Open))
33+
using (FileStream fs = new FileStream("myfile.txt", FileMode.Open)) // $ Source[cs/sql-injection]
3434
{
3535
using (StreamReader sr = new StreamReader(fs, Encoding.UTF8))
3636
{
@@ -42,7 +42,7 @@ public void RunSQLFromFile()
4242
continue;
4343
using (var connection = new SQLiteConnection(""))
4444
{
45-
var cmd = new SQLiteCommand(sql, connection);
45+
var cmd = new SQLiteCommand(sql, connection); // $ Alert[cs/sql-injection]
4646
cmd.ExecuteScalar();
4747
}
4848
}

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ public void GetDataSetByCategory()
3535
using (var connection = new SqlConnection(connectionString))
3636
{
3737
var query1 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
38-
+ categoryTextBox.Text + "' ORDER BY PRICE";
39-
var adapter = new SqlDataAdapter(query1, connection);
38+
+ categoryTextBox.Text + "' ORDER BY PRICE"; // $ Source[cs/sql-injection]
39+
var adapter = new SqlDataAdapter(query1, connection); // $ Alert[cs/sql-injection]
4040
var result = new DataSet();
4141
adapter.Fill(result);
4242
}
@@ -70,9 +70,9 @@ public void GetDataSetByCategory()
7070
{
7171
// BAD: Use EntityFramework direct Sql execution methods
7272
var query1 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
73-
+ categoryTextBox.Text + "' ORDER BY PRICE";
74-
context.Database.ExecuteSqlCommand(query1);
75-
context.Database.SqlQuery<string>(query1);
73+
+ categoryTextBox.Text + "' ORDER BY PRICE"; // $ Source[cs/sql-injection]
74+
context.Database.ExecuteSqlCommand(query1); // $ Alert[cs/sql-injection]
75+
context.Database.SqlQuery<string>(query1); // $ Alert[cs/sql-injection]
7676
// GOOD: Use EntityFramework direct Sql execution methods with parameter
7777
var query2 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY="
7878
+ "@p0 ORDER BY PRICE";
@@ -84,8 +84,8 @@ public void GetDataSetByCategory()
8484
using (var connection = new SqlConnection(connectionString))
8585
{
8686
var query1 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
87-
+ box1.Text + "' ORDER BY PRICE";
88-
var adapter = new SqlDataAdapter(query1, connection);
87+
+ box1.Text + "' ORDER BY PRICE"; // $ Source[cs/sql-injection]
88+
var adapter = new SqlDataAdapter(query1, connection); // $ Alert[cs/sql-injection]
8989
var result = new DataSet();
9090
adapter.Fill(result);
9191
}
@@ -94,9 +94,9 @@ public void GetDataSetByCategory()
9494
using (var connection = new SqlConnection(connectionString))
9595
{
9696
var queryString = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
97-
+ box1.Text + "' ORDER BY PRICE";
98-
var cmd = new SqlCommand(queryString);
99-
var adapter = new SqlDataAdapter(cmd);
97+
+ box1.Text + "' ORDER BY PRICE"; // $ Source[cs/sql-injection]
98+
var cmd = new SqlCommand(queryString); // $ Alert[cs/sql-injection]
99+
var adapter = new SqlDataAdapter(cmd); // $ Alert[cs/sql-injection]
100100
var result = new DataSet();
101101
adapter.Fill(result);
102102
}
@@ -105,9 +105,9 @@ public void GetDataSetByCategory()
105105
using (var connection = new SqlConnection(connectionString))
106106
{
107107
var queryString = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
108-
+ Console.ReadLine()! + "' ORDER BY PRICE";
109-
var cmd = new SqlCommand(queryString);
110-
var adapter = new SqlDataAdapter(cmd);
108+
+ Console.ReadLine()! + "' ORDER BY PRICE"; // $ Source[cs/sql-injection]
109+
var cmd = new SqlCommand(queryString); // $ Alert[cs/sql-injection]
110+
var adapter = new SqlDataAdapter(cmd); // $ Alert[cs/sql-injection]
111111
var result = new DataSet();
112112
adapter.Fill(result);
113113
}
@@ -119,14 +119,14 @@ public void GetDataSetByCategory()
119119
public abstract class MyController : Controller
120120
{
121121
[HttpPost("{userId:string}")]
122-
public async Task<IActionResult> GetUserById([FromRoute] string userId, CancellationToken cancellationToken)
122+
public async Task<IActionResult> GetUserById([FromRoute] string userId, CancellationToken cancellationToken) // $ Source[cs/sql-injection]
123123
{
124124
// This is a vulnerable method due to SQL injection
125125
string query = "SELECT * FROM Users WHERE UserId = '" + userId + "'";
126126

127127
using (SqlConnection connection = new SqlConnection("YourConnectionString"))
128128
{
129-
SqlCommand command = new SqlCommand(query, connection);
129+
SqlCommand command = new SqlCommand(query, connection); // $ Alert[cs/sql-injection]
130130
connection.Open();
131131

132132
SqlDataReader reader = command.ExecuteReader();

0 commit comments

Comments
 (0)