Skip to content

Commit 1590633

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #9923 from michaelnebel/csharp/webgoat
C#: SQL Injection improvements for SQLite.
2 parents 79a7164 + 9cb4e4a commit 1590633

File tree

19 files changed

+2064
-14
lines changed

19 files changed

+2064
-14
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ module CsvValidation {
383383
or
384384
exists(string row, string kind | sourceModel(row) |
385385
kind = row.splitAt(";", 7) and
386-
not kind = "local" and
386+
not kind = ["local", "file"] and
387387
msg = "Invalid kind \"" + kind + "\" in source model."
388388
)
389389
}

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

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ class IDbCommandConstructionSqlExpr extends SqlExpr, ObjectCreation {
3939
.hasQualifiedName([
4040
// Known sealed classes:
4141
"System.Data.SqlClient.SqlCommand", "System.Data.Odbc.OdbcCommand",
42-
"System.Data.OleDb.OleDbCommand", "System.Data.EntityClient.EntityCommand"
42+
"System.Data.OleDb.OleDbCommand", "System.Data.EntityClient.EntityCommand",
43+
"System.Data.SQLite.SQLiteCommand"
4344
])
4445
)
4546
}
@@ -67,18 +68,46 @@ private class IDbCommandConstructionSinkModelCsv extends SinkModelCsv {
6768
// EntityCommand
6869
"System.Data.EntityClient;EntityCommand;false;EntityCommand;(System.String);;Argument[0];sql;manual",
6970
"System.Data.EntityClient;EntityCommand;false;EntityCommand;(System.String,System.Data.EntityClient.EntityConnection);;Argument[0];sql;manual",
70-
"System.Data.EntityClient;EntityCommand;false;EntityCommand;(System.String,System.Data.EntityClient.EntityConnection,System.Data.EntityClient.EntityTransaction);;Argument[0];sql;manual"
71+
"System.Data.EntityClient;EntityCommand;false;EntityCommand;(System.String,System.Data.EntityClient.EntityConnection,System.Data.EntityClient.EntityTransaction);;Argument[0];sql;manual",
72+
// SQLiteCommand
73+
"System.Data.SQLite;SQLiteCommand;false;SQLiteCommand;(System.String);;Argument[0];sql;manual",
74+
"System.Data.SQLite;SQLiteCommand;false;SQLiteCommand;(System.String,System.Data.SQLite.SQLiteConnection);;Argument[0];sql;manual",
75+
"System.Data.SQLite;SQLiteCommand;false;SQLiteCommand;(System.String,System.Data.SQLite.SQLiteConnection,System.Data.SQLite.SQLiteTransaction);;Argument[0];sql;manual",
7176
]
7277
}
7378
}
7479

75-
/** A construction of an `SqlDataAdapter` object. */
80+
/** Data flow for SqlCommand and friends. */
81+
private class SqlCommandSummaryModelCsv extends SummaryModelCsv {
82+
override predicate row(string row) {
83+
row =
84+
[
85+
// SqlCommand
86+
"System.Data.SqlClient;SqlCommand;false;SqlCommand;(System.String);;Argument[0];Argument[Qualifier];taint;manual",
87+
"System.Data.SqlClient;SqlCommand;false;SqlCommand;(System.String,System.Data.SqlClient.SqlConnection);;Argument[0];Argument[Qualifier];taint;manual",
88+
"System.Data.SqlClient;SqlCommand;false;SqlCommand;(System.String,System.Data.SqlClient.SqlConnection,System.Data.SqlClient.SqlTransaction);;Argument[0];Argument[Qualifier];taint;manual",
89+
// SQLiteCommand.
90+
"System.Data.SQLite;SQLiteCommand;false;SQLiteCommand;(System.String);;Argument[0];Argument[Qualifier];taint;manual",
91+
"System.Data.SQLite;SQLiteCommand;false;SQLiteCommand;(System.String,System.Data.SQLite.SQLiteConnection);;Argument[0];Argument[Qualifier];taint;manual",
92+
"System.Data.SQLite;SQLiteCommand;false;SQLiteCommand;(System.String,System.Data.SQLite.SQLiteConnection,System.Data.SQLite.SQLiteTransaction);;Argument[0];Argument[Qualifier];taint;manual",
93+
]
94+
}
95+
}
96+
97+
/** A construction of an `Adapter` object. */
7698
private class SqlDataAdapterConstructionSinkModelCsv extends SinkModelCsv {
7799
override predicate row(string row) {
78100
row =
79101
[
102+
// SqlDataAdapter
103+
"System.Data.SqlClient;SqlDataAdapter;false;SqlDataAdapter;(System.Data.SqlClient.SqlCommand);;Argument[0];sql;manual",
80104
"System.Data.SqlClient;SqlDataAdapter;false;SqlDataAdapter;(System.String,System.String);;Argument[0];sql;manual",
81-
"System.Data.SqlClient;SqlDataAdapter;false;SqlDataAdapter;(System.String,System.Data.SqlClient.SqlConnection);;Argument[0];sql;manual"
105+
"System.Data.SqlClient;SqlDataAdapter;false;SqlDataAdapter;(System.String,System.Data.SqlClient.SqlConnection);;Argument[0];sql;manual",
106+
// SQLiteDataAdapter
107+
"System.Data.SQLite;SQLiteDataAdapter;false;SQLiteDataAdapter;(System.Data.SQLite.SQLiteCommand);;Argument[0];sql;manual",
108+
"System.Data.SQLite;SQLiteDataAdapter;false;SQLiteDataAdapter;(System.String,System.Data.SQLite.SQLiteConnection);;Argument[0];sql;manual",
109+
"System.Data.SQLite;SQLiteDataAdapter;false;SQLiteDataAdapter;(System.String,System.String);;Argument[0];sql;manual",
110+
"System.Data.SQLite;SQLiteDataAdapter;false;SQLiteDataAdapter;(System.String,System.String,System.Boolean);;Argument[0];sql;manual",
82111
]
83112
}
84113
}

‎csharp/ql/lib/semmle/code/csharp/frameworks/system/IO.qll‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ class SystemIOMemoryStreamClass extends SystemIOClass {
179179
}
180180
}
181181

182+
/** Data flow for `System.IO.MemoryStream`. */
182183
private class SystemIOMemoryStreamFlowModelCsv extends SummaryModelCsv {
183184
override predicate row(string row) {
184185
row =
@@ -192,3 +193,32 @@ private class SystemIOMemoryStreamFlowModelCsv extends SummaryModelCsv {
192193
]
193194
}
194195
}
196+
197+
/** Sources for `System.IO.FileStream`. */
198+
private class SystemIOFileStreamSourceModelCsv extends SourceModelCsv {
199+
override predicate row(string row) {
200+
row = "System.IO;FileStream;false;FileStream;;;Argument[Qualifier];file;manual"
201+
}
202+
}
203+
204+
/** Data flow for `System.IO.FileStream`. */
205+
private class SystemIOFileStreamSummaryModelCsv extends SummaryModelCsv {
206+
override predicate row(string row) {
207+
row =
208+
[
209+
"System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32,System.IO.FileOptions);;Argument[0];Argument[Qualifier];taint;manual",
210+
"System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32);;Argument[0];Argument[Qualifier];taint;manual",
211+
"System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare);;Argument[0];Argument[Qualifier];taint;manual",
212+
"System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual",
213+
"System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess);;Argument[0];Argument[Qualifier];taint;manual",
214+
"System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode);;Argument[0];Argument[Qualifier];taint;manual",
215+
]
216+
}
217+
}
218+
219+
/** Data flow for `System.IO.StreamReader`. */
220+
private class SystemIOStreamSummaryModelCsv extends SummaryModelCsv {
221+
override predicate row(string row) {
222+
row = "System.IO;StreamReader;false;StreamReader;;;Argument[0];Argument[Qualifier];taint;manual"
223+
}
224+
}

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Stored.qll‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import csharp
6+
private import semmle.code.csharp.dataflow.ExternalFlow
67
private import semmle.code.csharp.frameworks.system.data.Common
78
private import semmle.code.csharp.frameworks.system.data.Entity
89
private import semmle.code.csharp.frameworks.EntityFramework
@@ -55,3 +56,8 @@ class ORMMappedProperty extends StoredFlowSource {
5556
this instanceof NHibernate::StoredFlowSource
5657
}
5758
}
59+
60+
/** A file stream source is considered a stored flow source. */
61+
class FileStreamStoredFlowSource extends StoredFlowSource {
62+
FileStreamStoredFlowSource() { sourceNode(this, "file") }
63+
}

‎csharp/ql/src/change-notes/2022-08-10-sqlinjection-queries.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added better support for the SQLite framework in the SQL injection query.
5+
* File streams are now considered stored flow sources. Eg. reading query elements from a file can lead to a Second Order SQL injection alert.

‎csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3542,6 +3542,12 @@
35423542
| System.IO;FileStream;false;BeginRead;(System.Byte[],System.Int32,System.Int32,System.AsyncCallback,System.Object);;Argument[Qualifier];Argument[0].Element;taint;manual |
35433543
| System.IO;FileStream;false;BeginWrite;(System.Byte[],System.Int32,System.Int32,System.AsyncCallback,System.Object);;Argument[0].Element;Argument[Qualifier];taint;manual |
35443544
| System.IO;FileStream;false;CopyToAsync;(System.IO.Stream,System.Int32,System.Threading.CancellationToken);;Argument[Qualifier];Argument[0];taint;manual |
3545+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode);;Argument[0];Argument[Qualifier];taint;manual |
3546+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess);;Argument[0];Argument[Qualifier];taint;manual |
3547+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare);;Argument[0];Argument[Qualifier];taint;manual |
3548+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32);;Argument[0];Argument[Qualifier];taint;manual |
3549+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
3550+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32,System.IO.FileOptions);;Argument[0];Argument[Qualifier];taint;manual |
35453551
| System.IO;FileStream;false;FlushAsync;(System.Threading.CancellationToken);;Argument[0];ReturnValue;taint;generated |
35463552
| System.IO;FileStream;false;Read;(System.Byte[],System.Int32,System.Int32);;Argument[Qualifier];Argument[0].Element;taint;manual |
35473553
| System.IO;FileStream;false;ReadAsync;(System.Byte[],System.Int32,System.Int32,System.Threading.CancellationToken);;Argument[Qualifier];Argument[0].Element;taint;manual |
@@ -3668,8 +3674,19 @@
36683674
| System.IO;StreamReader;false;ReadLineAsync;();;Argument[Qualifier];ReturnValue;taint;manual |
36693675
| System.IO;StreamReader;false;ReadToEnd;();;Argument[Qualifier];ReturnValue;taint;manual |
36703676
| System.IO;StreamReader;false;ReadToEndAsync;();;Argument[Qualifier];ReturnValue;taint;manual |
3671-
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;generated |
3672-
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32,System.Boolean);;Argument[1];Argument[Qualifier];taint;generated |
3677+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream);;Argument[0];Argument[Qualifier];taint;manual |
3678+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
3679+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding);;Argument[0];Argument[Qualifier];taint;manual |
3680+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
3681+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32);;Argument[0];Argument[Qualifier];taint;manual |
3682+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
3683+
| System.IO;StreamReader;false;StreamReader;(System.String);;Argument[0];Argument[Qualifier];taint;manual |
3684+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
3685+
| System.IO;StreamReader;false;StreamReader;(System.String,System.IO.FileStreamOptions);;Argument[0];Argument[Qualifier];taint;manual |
3686+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding);;Argument[0];Argument[Qualifier];taint;manual |
3687+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
3688+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding,System.Boolean,System.IO.FileStreamOptions);;Argument[0];Argument[Qualifier];taint;manual |
3689+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding,System.Boolean,System.Int32);;Argument[0];Argument[Qualifier];taint;manual |
36733690
| System.IO;StreamReader;false;get_BaseStream;();;Argument[Qualifier];ReturnValue;taint;generated |
36743691
| System.IO;StreamReader;false;get_CurrentEncoding;();;Argument[Qualifier];ReturnValue;taint;generated |
36753692
| System.IO;StreamWriter;false;StreamWriter;(System.IO.Stream,System.Text.Encoding,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;generated |

‎csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2714,6 +2714,12 @@
27142714
| System.IO;FileNotFoundException;false;GetObjectData;(System.Runtime.Serialization.SerializationInfo,System.Runtime.Serialization.StreamingContext);;Argument[Qualifier];Argument[0];taint;generated |
27152715
| System.IO;FileNotFoundException;false;ToString;();;Argument[Qualifier];ReturnValue;taint;generated |
27162716
| System.IO;FileNotFoundException;false;get_Message;();;Argument[Qualifier];ReturnValue;taint;generated |
2717+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode);;Argument[0];Argument[Qualifier];taint;manual |
2718+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess);;Argument[0];Argument[Qualifier];taint;manual |
2719+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare);;Argument[0];Argument[Qualifier];taint;manual |
2720+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32);;Argument[0];Argument[Qualifier];taint;manual |
2721+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
2722+
| System.IO;FileStream;false;FileStream;(System.String,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare,System.Int32,System.IO.FileOptions);;Argument[0];Argument[Qualifier];taint;manual |
27172723
| System.IO;FileStream;false;FlushAsync;(System.Threading.CancellationToken);;Argument[0];ReturnValue;taint;generated |
27182724
| System.IO;FileStream;false;ReadAsync;(System.Memory<System.Byte>,System.Threading.CancellationToken);;Argument[1];ReturnValue;taint;generated |
27192725
| System.IO;FileStream;false;ReadAsync;(System.Memory<System.Byte>,System.Threading.CancellationToken);;Argument[Qualifier];ReturnValue;taint;generated |
@@ -2815,8 +2821,19 @@
28152821
| System.IO;Stream;true;ReadAsync;(System.Byte[],System.Int32,System.Int32,System.Threading.CancellationToken);;Argument[Qualifier];Argument[0].Element;taint;manual |
28162822
| System.IO;Stream;true;Write;(System.Byte[],System.Int32,System.Int32);;Argument[0].Element;Argument[Qualifier];taint;manual |
28172823
| System.IO;Stream;true;WriteAsync;(System.Byte[],System.Int32,System.Int32,System.Threading.CancellationToken);;Argument[0].Element;Argument[Qualifier];taint;manual |
2818-
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;generated |
2819-
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32,System.Boolean);;Argument[1];Argument[Qualifier];taint;generated |
2824+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream);;Argument[0];Argument[Qualifier];taint;manual |
2825+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
2826+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding);;Argument[0];Argument[Qualifier];taint;manual |
2827+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
2828+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32);;Argument[0];Argument[Qualifier];taint;manual |
2829+
| System.IO;StreamReader;false;StreamReader;(System.IO.Stream,System.Text.Encoding,System.Boolean,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
2830+
| System.IO;StreamReader;false;StreamReader;(System.String);;Argument[0];Argument[Qualifier];taint;manual |
2831+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
2832+
| System.IO;StreamReader;false;StreamReader;(System.String,System.IO.FileStreamOptions);;Argument[0];Argument[Qualifier];taint;manual |
2833+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding);;Argument[0];Argument[Qualifier];taint;manual |
2834+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding,System.Boolean);;Argument[0];Argument[Qualifier];taint;manual |
2835+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding,System.Boolean,System.IO.FileStreamOptions);;Argument[0];Argument[Qualifier];taint;manual |
2836+
| System.IO;StreamReader;false;StreamReader;(System.String,System.Text.Encoding,System.Boolean,System.Int32);;Argument[0];Argument[Qualifier];taint;manual |
28202837
| System.IO;StreamReader;false;get_BaseStream;();;Argument[Qualifier];ReturnValue;taint;generated |
28212838
| System.IO;StreamReader;false;get_CurrentEncoding;();;Argument[Qualifier];ReturnValue;taint;generated |
28222839
| System.IO;StreamWriter;false;StreamWriter;(System.IO.Stream,System.Text.Encoding,System.Int32,System.Boolean);;Argument[0];Argument[Qualifier];taint;generated |

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44
namespace Test
55
{
66

7+
using System.Data.SQLite;
8+
using System.IO;
9+
using System.Text;
10+
711
class SecondOrderSqlInjection
812
{
913

10-
public void processRequest()
14+
public void ProcessRequest()
1115
{
1216
using (SqlConnection connection = new SqlConnection(""))
1317
{
@@ -23,5 +27,28 @@ public void processRequest()
2327
customerReader.Close();
2428
}
2529
}
30+
31+
public void RunSQLFromFile()
32+
{
33+
using (FileStream fs = new FileStream("myfile.txt", FileMode.Open))
34+
{
35+
using (StreamReader sr = new StreamReader(fs, Encoding.UTF8))
36+
{
37+
var sql = String.Empty;
38+
while ((sql = sr.ReadLine()) != null)
39+
{
40+
sql = sql.Trim();
41+
if (sql.StartsWith("--"))
42+
continue;
43+
using (var connection = new SQLiteConnection(""))
44+
{
45+
var cmd = new SQLiteCommand(sql, connection);
46+
cmd.ExecuteScalar();
47+
}
48+
}
49+
}
50+
}
51+
}
52+
2653
}
2754
}

0 commit comments

Comments
 (0)