mirror of
https://github.com/github/codeql.git
synced 2025-12-24 04:36:35 +01:00
Merge pull request #12658 from joefarebrother/csharp-sensitive-data
C#: Add local filesystem writes as External Location sinks
This commit is contained in:
Joe Farebrother
@@ -4,6 +4,23 @@ extensions:
|
||||
extensible: sourceModel
|
||||
data:
|
||||
- ["System.IO", "FileStream", False, "FileStream", "", "", "Argument[this]", "file", "manual"]
|
||||
- ["System.IO", "FileStream", False, "FileStream", "", "", "Argument[this]", "file-write", "manual"]
|
||||
- ["System.IO", "StreamWriter", False, "StreamWriter", "(System.String)", "", "Argument[this]", "file-write", "manual"]
|
||||
- ["System.IO", "StreamWriter", False, "StreamWriter", "(System.String,System.Boolean)", "", "Argument[this]", "file-write", "manual"]
|
||||
- ["System.IO", "StreamWriter", False, "StreamWriter", "(System.String,System.Boolean,System.Text.Encoding)", "", "Argument[this]", "file-write", "manual"]
|
||||
- ["System.IO", "StreamWriter", False, "StreamWriter", "(System.String,System.Boolean,System.Text.Encoding,System.Int32)", "", "Argument[this]", "file-write", "manual"]
|
||||
- ["System.IO", "StreamWriter", False, "StreamWriter", "(System.String,System.Text.Encoding,System.IO.FileStreamOptions)", "", "Argument[this]", "file-write", "manual"]
|
||||
- ["System.IO", "StreamWriter", False, "StreamWriter", "(System.String,System.IO.FileStreamOptions)", "", "Argument[this]", "file-write", "manual"]
|
||||
- ["System.IO", "File", False, "Open", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "File", False, "OpenWrite", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "File", False, "Create", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "File", False, "CreateText", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "File", False, "AppendText", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "FileInfo", False, "Open", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "FileInfo", False, "OpenWrite", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "FileInfo", False, "Create", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "FileInfo", False, "CreateText", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- ["System.IO", "FileInfo", False, "AppendText", "", "", "ReturnValue", "file-write", "manual"]
|
||||
- addsTo:
|
||||
pack: codeql/csharp-all
|
||||
extensible: summaryModel
|
||||
|
||||
@@ -215,7 +215,7 @@ module ModelValidation {
|
||||
)
|
||||
or
|
||||
exists(string kind | sourceModel(_, _, _, _, _, _, _, kind, _) |
|
||||
not kind = ["local", "remote", "file"] and
|
||||
not kind = ["local", "remote", "file", "file-write"] and
|
||||
result = "Invalid kind \"" + kind + "\" in source model."
|
||||
)
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import csharp
|
||||
private import Remote
|
||||
private import semmle.code.csharp.commons.Loggers
|
||||
private import semmle.code.csharp.frameworks.system.Web
|
||||
private import semmle.code.csharp.frameworks.system.IO
|
||||
private import semmle.code.csharp.dataflow.ExternalFlow
|
||||
|
||||
/**
|
||||
@@ -63,3 +64,56 @@ class CookieStorageSink extends ExternalLocationSink, RemoteFlowSink {
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private predicate isFileWriteCall(Expr stream, Expr data) {
|
||||
exists(MethodCall mc, Method m | mc.getTarget() = m.getAnOverrider*() |
|
||||
m.hasQualifiedName("System.IO", "Stream", ["Write", "WriteAsync"]) and
|
||||
stream = mc.getQualifier() and
|
||||
data = mc.getArgument(0)
|
||||
or
|
||||
m.hasQualifiedName("System.IO", "TextWriter",
|
||||
["Write", "WriteAsync", "WriteLine", "WriteLineAsync"]) and
|
||||
stream = mc.getQualifier() and
|
||||
data = mc.getArgument(0)
|
||||
or
|
||||
m.hasQualifiedName("System.Xml.Linq", "XDocument", ["Save", "SaveAsync"]) and
|
||||
data = mc.getQualifier() and
|
||||
stream = mc.getArgument(0)
|
||||
)
|
||||
}
|
||||
|
||||
/** A configuration for tracking flow from calls that open a file in write mode to methods that write to that file, excluding encrypted streams. */
|
||||
private module LocalFileOutputStreamConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node src) { sourceNode(src, "file-write") }
|
||||
|
||||
predicate isSink(DataFlow::Node sink) { isFileWriteCall(sink.asExpr(), _) }
|
||||
|
||||
predicate isBarrier(DataFlow::Node node) {
|
||||
node.asExpr()
|
||||
.(ObjectCreation)
|
||||
.getObjectType()
|
||||
.hasQualifiedName("System.Security.Cryptography", "CryptoStream")
|
||||
}
|
||||
|
||||
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
exists(ObjectCreation oc |
|
||||
node2.asExpr() = oc and
|
||||
node1.asExpr() = oc.getArgument(0) and
|
||||
oc.getObjectType() instanceof SystemIOStreamWriterClass
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private module LocalFileOutputStreamFlow = DataFlow::Global<LocalFileOutputStreamConfig>;
|
||||
|
||||
/**
|
||||
* A write to the local filesystem.
|
||||
*/
|
||||
class LocalFileOutputSink extends ExternalLocationSink {
|
||||
LocalFileOutputSink() {
|
||||
exists(DataFlow::Node streamSink |
|
||||
LocalFileOutputStreamFlow::flowTo(streamSink) and
|
||||
isFileWriteCall(streamSink.asExpr(), this.asExpr())
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Additional sinks modelling writes to unencrypted local files have been added to `ExternalLocationSink`, used by the `cs/cleartext-storage` and `cs/exposure-of-sensitive-information` queries.
|
||||
@@ -2,6 +2,8 @@ using System.Text;
|
||||
using System.Web;
|
||||
using System.Web.Security;
|
||||
using System.Windows.Forms;
|
||||
using System.IO;
|
||||
using System.Security.Cryptography;
|
||||
|
||||
public class ClearTextStorageHandler : IHttpHandler
|
||||
{
|
||||
@@ -24,6 +26,22 @@ public class ClearTextStorageHandler : IHttpHandler
|
||||
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)
|
||||
@@ -31,6 +49,10 @@ public class ClearTextStorageHandler : IHttpHandler
|
||||
return Encoding.UTF8.GetString(MachineKey.Protect(Encoding.UTF8.GetBytes(value), type));
|
||||
}
|
||||
|
||||
public ICryptoTransform GetEncryptor(){
|
||||
return null;
|
||||
}
|
||||
|
||||
public string GetPassword()
|
||||
{
|
||||
return "password";
|
||||
|
||||
@@ -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 |
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
using System.Web;
|
||||
using System.Security.Cryptography;
|
||||
using System.IO;
|
||||
|
||||
public class Person
|
||||
{
|
||||
@@ -21,9 +23,29 @@ public class ExposureOfPrivateInformationHandler : IHttpHandler
|
||||
ILogger logger = new ILogger();
|
||||
logger.Warn(p.getTelephone());
|
||||
|
||||
// BAD: Storing sensitive data in unencrypted local file
|
||||
using (var writeStream = File.Open("telephones.txt", FileMode.Create))
|
||||
{
|
||||
var writer = new StreamWriter(writeStream);
|
||||
writer.Write(p.getTelephone());
|
||||
writer.Close();
|
||||
}
|
||||
|
||||
// GOOD: Storing encrypted sensitive data
|
||||
using (var writeStream = File.Open("telephones.txt", FileMode.Create))
|
||||
{
|
||||
var writer = new StreamWriter(new CryptoStream(writeStream, GetEncryptor(), CryptoStreamMode.Write));
|
||||
writer.Write(p.getTelephone());
|
||||
writer.Close();
|
||||
}
|
||||
|
||||
// GOOD: Don't write these values to sensitive locations in the first place
|
||||
}
|
||||
|
||||
public ICryptoTransform GetEncryptor(){
|
||||
return null;
|
||||
}
|
||||
|
||||
public bool IsReusable
|
||||
{
|
||||
get
|
||||
|
||||
@@ -1,12 +1,14 @@
|
||||
edges
|
||||
nodes
|
||||
| ExposureOfPrivateInformation.cs:16:50:16:84 | access to indexer | semmle.label | access to indexer |
|
||||
| ExposureOfPrivateInformation.cs:18:50:18:65 | call to method getTelephone | semmle.label | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:22:21:22:36 | call to method getTelephone | semmle.label | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:40:21:40:33 | access to property Text | semmle.label | access to property Text |
|
||||
| ExposureOfPrivateInformation.cs:18:50:18:84 | access to indexer | semmle.label | access to indexer |
|
||||
| ExposureOfPrivateInformation.cs:20:50:20:65 | call to method getTelephone | semmle.label | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:24:21:24:36 | call to method getTelephone | semmle.label | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:30:26:30:41 | call to method getTelephone | semmle.label | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:62:21:62:33 | access to property Text | semmle.label | access to property Text |
|
||||
subpaths
|
||||
#select
|
||||
| ExposureOfPrivateInformation.cs:16:50:16:84 | access to indexer | ExposureOfPrivateInformation.cs:16:50:16:84 | access to indexer | ExposureOfPrivateInformation.cs:16:50:16:84 | access to indexer | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:16:50:16:84 | access to indexer | access to indexer |
|
||||
| ExposureOfPrivateInformation.cs:18:50:18:65 | call to method getTelephone | ExposureOfPrivateInformation.cs:18:50:18:65 | call to method getTelephone | ExposureOfPrivateInformation.cs:18:50:18:65 | call to method getTelephone | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:18:50:18:65 | call to method getTelephone | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:22:21:22:36 | call to method getTelephone | ExposureOfPrivateInformation.cs:22:21:22:36 | call to method getTelephone | ExposureOfPrivateInformation.cs:22:21:22:36 | call to method getTelephone | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:22:21:22:36 | call to method getTelephone | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:40:21:40:33 | access to property Text | ExposureOfPrivateInformation.cs:40:21:40:33 | access to property Text | ExposureOfPrivateInformation.cs:40:21:40:33 | access to property Text | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:40:21:40:33 | access to property Text | access to property Text |
|
||||
| ExposureOfPrivateInformation.cs:18:50:18:84 | access to indexer | ExposureOfPrivateInformation.cs:18:50:18:84 | access to indexer | ExposureOfPrivateInformation.cs:18:50:18:84 | access to indexer | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:18:50:18:84 | access to indexer | access to indexer |
|
||||
| ExposureOfPrivateInformation.cs:20:50:20:65 | call to method getTelephone | ExposureOfPrivateInformation.cs:20:50:20:65 | call to method getTelephone | ExposureOfPrivateInformation.cs:20:50:20:65 | call to method getTelephone | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:20:50:20:65 | call to method getTelephone | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:24:21:24:36 | call to method getTelephone | ExposureOfPrivateInformation.cs:24:21:24:36 | call to method getTelephone | ExposureOfPrivateInformation.cs:24:21:24:36 | call to method getTelephone | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:24:21:24:36 | call to method getTelephone | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:30:26:30:41 | call to method getTelephone | ExposureOfPrivateInformation.cs:30:26:30:41 | call to method getTelephone | ExposureOfPrivateInformation.cs:30:26:30:41 | call to method getTelephone | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:30:26:30:41 | call to method getTelephone | call to method getTelephone |
|
||||
| ExposureOfPrivateInformation.cs:62:21:62:33 | access to property Text | ExposureOfPrivateInformation.cs:62:21:62:33 | access to property Text | ExposureOfPrivateInformation.cs:62:21:62:33 | access to property Text | Private data returned by $@ is written to an external location. | ExposureOfPrivateInformation.cs:62:21:62:33 | access to property Text | access to property Text |
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user