Add test for cleartext storage

joefarebrother · joefarebrother · commit 941df4f2748e · 2023-04-05T13:57:23.000+01:00
diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll
@@ -65,19 +65,21 @@ class CookieStorageSink extends ExternalLocationSink, RemoteFlowSink {
   }
 }
 
+
+
 private predicate isFileWriteCall(Expr stream, Expr data) {
   exists(MethodCall mc, Method m | mc.getTarget() = m.getAnOverrider*() |
-    mc.getTarget().hasQualifiedName("System.IO", "Stream", ["Write", "WriteAsync"]) and
+    m.hasQualifiedName("System.IO", "Stream", ["Write", "WriteAsync"]) and
     stream = mc.getQualifier() and
     data = mc.getArgument(0)
     or
-    mc.getTarget()
+    m
         .hasQualifiedName("System.IO", "TextWriter",
           ["Write", "WriteAsync", "WriteLine", "WriteLineAsync"]) and
     stream = mc.getQualifier() and
     data = mc.getArgument(0)
     or
-    mc.getTarget().hasQualifiedName("System.Xml.Linq", "XDocument", ["Save", "SaveAsync"]) and
+    m.hasQualifiedName("System.Xml.Linq", "XDocument", ["Save", "SaveAsync"]) and
     data = mc.getQualifier() and
     stream = mc.getArgument(0)
   )
@@ -117,15 +119,15 @@ private module LocalFileOutputStreamConfig implements DataFlow::ConfigSig {
   }
 }
 
-private module LocalFileOutputStreamFlow = DataFlow::Make<LocalFileOutputStreamConfig>;
+private module LocalFileOutputStreamFlow = DataFlow::Global<LocalFileOutputStreamConfig>;
 
 /**
  * A write to the local filesystem.
  */
 class LocalFileOutputSink extends ExternalLocationSink {
   LocalFileOutputSink() {
     exists(DataFlow::Node streamSink |
-      LocalFileOutputStreamFlow::hasFlow(_, streamSink) and
+      LocalFileOutputStreamFlow::flow(_, streamSink) and
       isFileWriteCall(streamSink.asExpr(), this.asExpr())
     )
   }
diff --git a/csharp/ql/test/query-tests/Security Features/CWE-312/CleartextStorage.cs b/csharp/ql/test/query-tests/Security Features/CWE-312/CleartextStorage.cs
@@ -2,6 +2,8 @@
 using System.Web;
 using System.Web.Security;
 using System.Windows.Forms;
+using System.IO;
+using System.Security.Cryptography;
 
 public class ClearTextStorageHandler : IHttpHandler
 {
@@ -24,13 +26,33 @@ public void ProcessRequest(HttpContext ctx)
         logger.Warn(GetPassword());
         // GOOD: Logging encrypted sensitive data
         logger.Warn(Encode(GetPassword(), "Password"));
+
+        // BAD: Storing sensitive data in local file 
+        using (var writeStream = File.Open("passwords.txt", FileMode.Create))
+        {
+            var writer = new StreamWriter(writeStream);
+            writer.Write(GetPassword());
+            writer.Close();
+        }
+
+        // GOOD: Storing encrypted sensitive data
+        using (var writeStream = File.Open("passwords.txt", FileMode.Create))
+        {
+            var writer = new StreamWriter(new CryptoStream(writeStream, GetEncryptor(), CryptoStreamMode.Write));
+            writer.Write(GetPassword());
+            writer.Close();
+        }
     }
 
     public string Encode(string value, string type)
     {
         return Encoding.UTF8.GetString(MachineKey.Protect(Encoding.UTF8.GetBytes(value), type));
     }
 
+    public ICryptoTransform GetEncryptor(){
+        return null;
+    }
+
     public string GetPassword()
     {
         return "password";
diff --git a/csharp/ql/test/query-tests/Security Features/CWE-312/CleartextStorage.expected b/csharp/ql/test/query-tests/Security Features/CWE-312/CleartextStorage.expected
@@ -1,20 +1,22 @@
 edges
 nodes
-| CleartextStorage.cs:13:50:13:59 | access to field accountKey | semmle.label | access to field accountKey |
-| CleartextStorage.cs:14:62:14:74 | call to method GetPassword | semmle.label | call to method GetPassword |
-| CleartextStorage.cs:15:69:15:81 | call to method GetPassword | semmle.label | call to method GetPassword |
-| CleartextStorage.cs:16:50:16:63 | call to method GetAccountID | semmle.label | call to method GetAccountID |
-| CleartextStorage.cs:24:21:24:33 | call to method GetPassword | semmle.label | call to method GetPassword |
-| CleartextStorage.cs:72:21:72:33 | access to property Text | semmle.label | access to property Text |
-| CleartextStorage.cs:73:21:73:29 | access to property Text | semmle.label | access to property Text |
-| CleartextStorage.cs:74:21:74:29 | access to property Text | semmle.label | access to property Text |
+| CleartextStorage.cs:15:50:15:59 | access to field accountKey | semmle.label | access to field accountKey |
+| CleartextStorage.cs:16:62:16:74 | call to method GetPassword | semmle.label | call to method GetPassword |
+| CleartextStorage.cs:17:69:17:81 | call to method GetPassword | semmle.label | call to method GetPassword |
+| CleartextStorage.cs:18:50:18:63 | call to method GetAccountID | semmle.label | call to method GetAccountID |
+| CleartextStorage.cs:26:21:26:33 | call to method GetPassword | semmle.label | call to method GetPassword |
+| CleartextStorage.cs:34:26:34:38 | call to method GetPassword | semmle.label | call to method GetPassword |
+| CleartextStorage.cs:94:21:94:33 | access to property Text | semmle.label | access to property Text |
+| CleartextStorage.cs:95:21:95:29 | access to property Text | semmle.label | access to property Text |
+| CleartextStorage.cs:96:21:96:29 | access to property Text | semmle.label | access to property Text |
 subpaths
 #select
-| CleartextStorage.cs:13:50:13:59 | access to field accountKey | CleartextStorage.cs:13:50:13:59 | access to field accountKey | CleartextStorage.cs:13:50:13:59 | access to field accountKey | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:13:50:13:59 | access to field accountKey | access to field accountKey |
-| CleartextStorage.cs:14:62:14:74 | call to method GetPassword | CleartextStorage.cs:14:62:14:74 | call to method GetPassword | CleartextStorage.cs:14:62:14:74 | call to method GetPassword | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:14:62:14:74 | call to method GetPassword | call to method GetPassword |
-| CleartextStorage.cs:15:69:15:81 | call to method GetPassword | CleartextStorage.cs:15:69:15:81 | call to method GetPassword | CleartextStorage.cs:15:69:15:81 | call to method GetPassword | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:15:69:15:81 | call to method GetPassword | call to method GetPassword |
-| CleartextStorage.cs:16:50:16:63 | call to method GetAccountID | CleartextStorage.cs:16:50:16:63 | call to method GetAccountID | CleartextStorage.cs:16:50:16:63 | call to method GetAccountID | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:16:50:16:63 | call to method GetAccountID | call to method GetAccountID |
-| CleartextStorage.cs:24:21:24:33 | call to method GetPassword | CleartextStorage.cs:24:21:24:33 | call to method GetPassword | CleartextStorage.cs:24:21:24:33 | call to method GetPassword | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:24:21:24:33 | call to method GetPassword | call to method GetPassword |
-| CleartextStorage.cs:72:21:72:33 | access to property Text | CleartextStorage.cs:72:21:72:33 | access to property Text | CleartextStorage.cs:72:21:72:33 | access to property Text | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:72:21:72:33 | access to property Text | access to property Text |
-| CleartextStorage.cs:73:21:73:29 | access to property Text | CleartextStorage.cs:73:21:73:29 | access to property Text | CleartextStorage.cs:73:21:73:29 | access to property Text | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:73:21:73:29 | access to property Text | access to property Text |
-| CleartextStorage.cs:74:21:74:29 | access to property Text | CleartextStorage.cs:74:21:74:29 | access to property Text | CleartextStorage.cs:74:21:74:29 | access to property Text | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:74:21:74:29 | access to property Text | access to property Text |
+| CleartextStorage.cs:15:50:15:59 | access to field accountKey | CleartextStorage.cs:15:50:15:59 | access to field accountKey | CleartextStorage.cs:15:50:15:59 | access to field accountKey | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:15:50:15:59 | access to field accountKey | access to field accountKey |
+| CleartextStorage.cs:16:62:16:74 | call to method GetPassword | CleartextStorage.cs:16:62:16:74 | call to method GetPassword | CleartextStorage.cs:16:62:16:74 | call to method GetPassword | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:16:62:16:74 | call to method GetPassword | call to method GetPassword |
+| CleartextStorage.cs:17:69:17:81 | call to method GetPassword | CleartextStorage.cs:17:69:17:81 | call to method GetPassword | CleartextStorage.cs:17:69:17:81 | call to method GetPassword | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:17:69:17:81 | call to method GetPassword | call to method GetPassword |
+| CleartextStorage.cs:18:50:18:63 | call to method GetAccountID | CleartextStorage.cs:18:50:18:63 | call to method GetAccountID | CleartextStorage.cs:18:50:18:63 | call to method GetAccountID | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:18:50:18:63 | call to method GetAccountID | call to method GetAccountID |
+| CleartextStorage.cs:26:21:26:33 | call to method GetPassword | CleartextStorage.cs:26:21:26:33 | call to method GetPassword | CleartextStorage.cs:26:21:26:33 | call to method GetPassword | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:26:21:26:33 | call to method GetPassword | call to method GetPassword |
+| CleartextStorage.cs:34:26:34:38 | call to method GetPassword | CleartextStorage.cs:34:26:34:38 | call to method GetPassword | CleartextStorage.cs:34:26:34:38 | call to method GetPassword | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:34:26:34:38 | call to method GetPassword | call to method GetPassword |
+| CleartextStorage.cs:94:21:94:33 | access to property Text | CleartextStorage.cs:94:21:94:33 | access to property Text | CleartextStorage.cs:94:21:94:33 | access to property Text | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:94:21:94:33 | access to property Text | access to property Text |
+| CleartextStorage.cs:95:21:95:29 | access to property Text | CleartextStorage.cs:95:21:95:29 | access to property Text | CleartextStorage.cs:95:21:95:29 | access to property Text | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:95:21:95:29 | access to property Text | access to property Text |
+| CleartextStorage.cs:96:21:96:29 | access to property Text | CleartextStorage.cs:96:21:96:29 | access to property Text | CleartextStorage.cs:96:21:96:29 | access to property Text | This stores sensitive data returned by $@ as clear text. | CleartextStorage.cs:96:21:96:29 | access to property Text | access to property Text |
diff --git a/csharp/ql/test/query-tests/Security Features/CWE-312/options b/csharp/ql/test/query-tests/Security Features/CWE-312/options
@@ -1 +1 @@
-semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.cs /r:System.Collections.Specialized.dll {testdir}/../../../../resources/stubs/System.Windows.cs
+semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.cs /r:System.Collections.Specialized.dll /r:System.Security.Cryptography.dll {testdir}/../../../../resources/stubs/System.Windows.cs

Original file line number	Diff line number	Diff line change
`@@ -65,19 +65,21 @@ class CookieStorageSink extends ExternalLocationSink, RemoteFlowSink {`
`65`	`65`	`}`
`66`	`66`	`}`
`67`	`67`
	`68`	`+`
	`69`	`+`
`68`	`70`	`private predicate isFileWriteCall(Expr stream, Expr data) {`
`69`	`71`	`exists(MethodCall mc, Method m \| mc.getTarget() = m.getAnOverrider*() \|`
`70`		`- mc.getTarget().hasQualifiedName("System.IO", "Stream", ["Write", "WriteAsync"]) and`
	`72`	`+ m.hasQualifiedName("System.IO", "Stream", ["Write", "WriteAsync"]) and`
`71`	`73`	`stream = mc.getQualifier() and`
`72`	`74`	`data = mc.getArgument(0)`
`73`	`75`	`or`
`74`		`- mc.getTarget()`
	`76`	`+ m`
`75`	`77`	`.hasQualifiedName("System.IO", "TextWriter",`
`76`	`78`	`["Write", "WriteAsync", "WriteLine", "WriteLineAsync"]) and`
`77`	`79`	`stream = mc.getQualifier() and`
`78`	`80`	`data = mc.getArgument(0)`
`79`	`81`	`or`
`80`		`- mc.getTarget().hasQualifiedName("System.Xml.Linq", "XDocument", ["Save", "SaveAsync"]) and`
	`82`	`+ m.hasQualifiedName("System.Xml.Linq", "XDocument", ["Save", "SaveAsync"]) and`
`81`	`83`	`data = mc.getQualifier() and`
`82`	`84`	`stream = mc.getArgument(0)`
`83`	`85`	`)`
`@@ -117,15 +119,15 @@ private module LocalFileOutputStreamConfig implements DataFlow::ConfigSig {`
`117`	`119`	`}`
`118`	`120`	`}`
`119`	`121`
`120`		`-private module LocalFileOutputStreamFlow = DataFlow::Make<LocalFileOutputStreamConfig>;`
	`122`	`+private module LocalFileOutputStreamFlow = DataFlow::Global<LocalFileOutputStreamConfig>;`
`121`	`123`
`122`	`124`	`/**`
`123`	`125`	`* A write to the local filesystem.`
`124`	`126`	`*/`
`125`	`127`	`class LocalFileOutputSink extends ExternalLocationSink {`
`126`	`128`	`LocalFileOutputSink() {`
`127`	`129`	`exists(DataFlow::Node streamSink \|`
`128`		`- LocalFileOutputStreamFlow::hasFlow(_, streamSink) and`
	`130`	`+ LocalFileOutputStreamFlow::flow(_, streamSink) and`
`129`	`131`	`isFileWriteCall(streamSink.asExpr(), this.asExpr())`
`130`	`132`	`)`
`131`	`133`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.cs /r:System.Collections.Specialized.dll {testdir}/../../../../resources/stubs/System.Windows.cs`
	`1`	`+semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.cs /r:System.Collections.Specialized.dll /r:System.Security.Cryptography.dll {testdir}/../../../../resources/stubs/System.Windows.cs`