Skip to content

Commit ffcb345

Browse files
committed
C#: Add Dapper support to SQL injection queries
1 parent 98001c4 commit ffcb345

File tree

4 files changed

+117
-0
lines changed

4 files changed

+117
-0
lines changed
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* Classes for modelling Dapper.
3+
*/
4+
5+
import csharp
6+
private import semmle.code.csharp.frameworks.system.Data
7+
8+
/** Definitions relating to the `Dapper` package. */
9+
module Dapper {
10+
/** The namespace `Dapper`. */
11+
class DapperNamespace extends Namespace {
12+
DapperNamespace() { this.hasQualifiedName("Dapper") }
13+
}
14+
15+
/** A class in `Dapper`. */
16+
class DapperClass extends Class {
17+
DapperClass() { this.getParent() instanceof DapperNamespace }
18+
}
19+
20+
/** A struct in `Dapper`. */
21+
class DapperStruct extends Struct {
22+
DapperStruct() { this.getParent() instanceof DapperNamespace }
23+
}
24+
25+
/** The `Dapper.SqlMapper` class. */
26+
class SqlMapperClass extends DapperClass {
27+
SqlMapperClass() { this.hasName("SqlMapper") }
28+
29+
/** Gets a DB query method. */
30+
ExtensionMethod getAQueryMethod() {
31+
result = this.getAMethod() and
32+
result.getName().regexpMatch("Query.*|Execute.*") and
33+
result.getExtendedType() instanceof SystemDataIDbConnectionInterface and
34+
result.isPublic()
35+
}
36+
}
37+
38+
/** The `Dapper.CommandDefinition` struct. */
39+
class CommandDefinitionStruct extends DapperStruct {
40+
CommandDefinitionStruct() { this.hasName("CommandDefinition") }
41+
}
42+
}

csharp/ql/src/semmle/code/csharp/frameworks/Sql.qll

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ private import semmle.code.csharp.frameworks.system.Data
55
private import semmle.code.csharp.frameworks.system.data.SqlClient
66
private import semmle.code.csharp.frameworks.EntityFramework
77
private import semmle.code.csharp.frameworks.NHibernate
8+
private import semmle.code.csharp.frameworks.Dapper
9+
private import semmle.code.csharp.dataflow.DataFlow3
810

911
/** An expression containing a SQL command. */
1012
abstract class SqlExpr extends Expr {
@@ -83,3 +85,37 @@ class MicrosoftSqlHelperMethodCallSqlExpr extends SqlExpr, MethodCall {
8385
)
8486
}
8587
}
88+
89+
/** A `Dapper.SqlMapper` method that is taking a SQL string argument. */
90+
class DapperSqlMethodCallSqlExpr extends SqlExpr, MethodCall {
91+
DapperSqlMethodCallSqlExpr() {
92+
this.getTarget() = any(Dapper::SqlMapperClass c).getAQueryMethod()
93+
}
94+
95+
override Expr getSql() { result = this.getArgumentForName("sql") }
96+
}
97+
98+
/** A `Dapper.CommandDefinition` creation that is taking a SQL string argument and is passed to a `Dapper.SqlMapper` method. */
99+
class DapperCommandDefinitionMethodCallSqlExpr extends SqlExpr, ObjectCreation {
100+
DapperCommandDefinitionMethodCallSqlExpr() {
101+
this.getObjectType() instanceof Dapper::CommandDefinitionStruct and
102+
exists(Conf c | c.hasFlow(DataFlow::exprNode(this), _))
103+
}
104+
105+
override Expr getSql() { result = this.getArgumentForName("commandText") }
106+
}
107+
108+
private class Conf extends DataFlow3::Configuration {
109+
Conf() { this = "DapperCommandDefinitionFlowConfig" }
110+
111+
override predicate isSource(DataFlow::Node node) {
112+
node.asExpr().(ObjectCreation).getObjectType() instanceof Dapper::CommandDefinitionStruct
113+
}
114+
115+
override predicate isSink(DataFlow::Node node) {
116+
exists(MethodCall mc |
117+
mc.getTarget() = any(Dapper::SqlMapperClass c).getAQueryMethod() and
118+
node.asExpr() = mc.getArgumentForName("command")
119+
)
120+
}
121+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ edges
55
| SqlInjection.cs:73:33:73:52 | access to property Text : String | SqlInjection.cs:74:56:74:61 | access to local variable query1 |
66
| SqlInjection.cs:73:33:73:52 | access to property Text : String | SqlInjection.cs:75:55:75:60 | access to local variable query1 |
77
| SqlInjection.cs:87:21:87:29 | access to property Text : String | SqlInjection.cs:88:50:88:55 | access to local variable query1 |
8+
| SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query |
9+
| SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query |
10+
| SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | SqlInjectionDapper.cs:39:63:39:67 | access to local variable query |
11+
| SqlInjectionDapper.cs:47:86:47:94 | access to property Text : String | SqlInjectionDapper.cs:49:47:49:51 | access to local variable query |
12+
| SqlInjectionDapper.cs:57:86:57:94 | access to property Text : String | SqlInjectionDapper.cs:58:42:58:46 | access to local variable query |
13+
| SqlInjectionDapper.cs:66:86:66:94 | access to property Text : String | SqlInjectionDapper.cs:67:42:67:46 | access to local variable query |
14+
| SqlInjectionDapper.cs:75:86:75:94 | access to property Text : String | SqlInjectionDapper.cs:77:52:77:56 | access to local variable query |
815
nodes
916
| SqlInjection.cs:38:21:38:35 | access to field categoryTextBox : TextBox | semmle.label | access to field categoryTextBox : TextBox |
1017
| SqlInjection.cs:38:21:38:40 | access to property Text : String | semmle.label | access to property Text : String |
@@ -15,8 +22,29 @@ nodes
1522
| SqlInjection.cs:75:55:75:60 | access to local variable query1 | semmle.label | access to local variable query1 |
1623
| SqlInjection.cs:87:21:87:29 | access to property Text : String | semmle.label | access to property Text : String |
1724
| SqlInjection.cs:88:50:88:55 | access to local variable query1 | semmle.label | access to local variable query1 |
25+
| SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | semmle.label | access to property Text : String |
26+
| SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | semmle.label | access to local variable query |
27+
| SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | semmle.label | access to property Text : String |
28+
| SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | semmle.label | access to local variable query |
29+
| SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | semmle.label | access to property Text : String |
30+
| SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | semmle.label | access to local variable query |
31+
| SqlInjectionDapper.cs:47:86:47:94 | access to property Text : String | semmle.label | access to property Text : String |
32+
| SqlInjectionDapper.cs:49:47:49:51 | access to local variable query | semmle.label | access to local variable query |
33+
| SqlInjectionDapper.cs:57:86:57:94 | access to property Text : String | semmle.label | access to property Text : String |
34+
| SqlInjectionDapper.cs:58:42:58:46 | access to local variable query | semmle.label | access to local variable query |
35+
| SqlInjectionDapper.cs:66:86:66:94 | access to property Text : String | semmle.label | access to property Text : String |
36+
| SqlInjectionDapper.cs:67:42:67:46 | access to local variable query | semmle.label | access to local variable query |
37+
| SqlInjectionDapper.cs:75:86:75:94 | access to property Text : String | semmle.label | access to property Text : String |
38+
| SqlInjectionDapper.cs:77:52:77:56 | access to local variable query | semmle.label | access to local variable query |
1839
#select
1940
| SqlInjection.cs:39:50:39:55 | access to local variable query1 | SqlInjection.cs:38:21:38:35 | access to field categoryTextBox : TextBox | SqlInjection.cs:39:50:39:55 | access to local variable query1 | Query might include code from $@. | SqlInjection.cs:38:21:38:35 | access to field categoryTextBox : TextBox | this ASP.NET user input |
2041
| SqlInjection.cs:74:56:74:61 | access to local variable query1 | SqlInjection.cs:73:33:73:47 | access to field categoryTextBox : TextBox | SqlInjection.cs:74:56:74:61 | access to local variable query1 | Query might include code from $@. | SqlInjection.cs:73:33:73:47 | access to field categoryTextBox : TextBox | this ASP.NET user input |
2142
| SqlInjection.cs:75:55:75:60 | access to local variable query1 | SqlInjection.cs:73:33:73:47 | access to field categoryTextBox : TextBox | SqlInjection.cs:75:55:75:60 | access to local variable query1 | Query might include code from $@. | SqlInjection.cs:73:33:73:47 | access to field categoryTextBox : TextBox | this ASP.NET user input |
2243
| SqlInjection.cs:88:50:88:55 | access to local variable query1 | SqlInjection.cs:87:21:87:29 | access to property Text : String | SqlInjection.cs:88:50:88:55 | access to local variable query1 | Query might include code from $@. | SqlInjection.cs:87:21:87:29 | access to property Text : String | this TextBox text |
44+
| SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | Query might include code from $@. | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | this TextBox text |
45+
| SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | Query might include code from $@. | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | this TextBox text |
46+
| SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | Query might include code from $@. | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | this TextBox text |
47+
| SqlInjectionDapper.cs:49:47:49:51 | access to local variable query | SqlInjectionDapper.cs:47:86:47:94 | access to property Text : String | SqlInjectionDapper.cs:49:47:49:51 | access to local variable query | Query might include code from $@. | SqlInjectionDapper.cs:47:86:47:94 | access to property Text : String | this TextBox text |
48+
| SqlInjectionDapper.cs:58:42:58:46 | access to local variable query | SqlInjectionDapper.cs:57:86:57:94 | access to property Text : String | SqlInjectionDapper.cs:58:42:58:46 | access to local variable query | Query might include code from $@. | SqlInjectionDapper.cs:57:86:57:94 | access to property Text : String | this TextBox text |
49+
| SqlInjectionDapper.cs:67:42:67:46 | access to local variable query | SqlInjectionDapper.cs:66:86:66:94 | access to property Text : String | SqlInjectionDapper.cs:67:42:67:46 | access to local variable query | Query might include code from $@. | SqlInjectionDapper.cs:66:86:66:94 | access to property Text : String | this TextBox text |
50+
| SqlInjectionDapper.cs:77:52:77:56 | access to local variable query | SqlInjectionDapper.cs:75:86:75:94 | access to property Text : String | SqlInjectionDapper.cs:77:52:77:56 | access to local variable query | Query might include code from $@. | SqlInjectionDapper.cs:75:86:75:94 | access to property Text : String | this TextBox text |

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,17 @@ public async Task Bad07()
7979
}
8080
}
8181

82+
public async Task Ok07()
83+
{
84+
using (var connection = new SqlConnection(connectionString))
85+
{
86+
var query = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" + box1.Text + "' ORDER BY PRICE";
87+
88+
var comDef = new CommandDefinition(query);
89+
// no call to any query method
90+
}
91+
}
92+
8293
System.Windows.Forms.TextBox box1;
8394
}
8495
}

0 commit comments

Comments
 (0)