diff --git a/csharp/ql/lib/ext/System.Web.model.yml b/csharp/ql/lib/ext/System.Web.model.yml index ba644e1cc70d..6d05cdae75b5 100644 --- a/csharp/ql/lib/ext/System.Web.model.yml +++ b/csharp/ql/lib/ext/System.Web.model.yml @@ -1,4 +1,10 @@ extensions: + - addsTo: + pack: codeql/csharp-all + extensible: barrierModel + data: + # The RawUrl property is considered to be safe for URL redirects + - ["System.Web", "HttpRequest", False, "get_RawUrl", "()", "", "ReturnValue", "url-redirection", "manual"] - addsTo: pack: codeql/csharp-all extensible: sinkModel diff --git a/csharp/ql/lib/ext/System.model.yml b/csharp/ql/lib/ext/System.model.yml index 870413e75698..963b37887eec 100644 --- a/csharp/ql/lib/ext/System.model.yml +++ b/csharp/ql/lib/ext/System.model.yml @@ -11,6 +11,11 @@ extensions: - ["System", "Environment", False, "get_CommandLine", "()", "", "ReturnValue", "commandargs", "manual"] - ["System", "Environment", False, "GetEnvironmentVariable", "", "", "ReturnValue", "environment", "manual"] - ["System", "Environment", False, "GetEnvironmentVariables", "", "", "ReturnValue", "environment", "manual"] + - addsTo: + pack: codeql/csharp-all + extensible: barrierGuardModel + data: + - ["System", "Uri", False, "get_IsAbsoluteUri", "()", "", "Argument[this]", "false", "url-redirection", "manual"] - addsTo: pack: codeql/csharp-all extensible: summaryModel diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CleartextStorageQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CleartextStorageQuery.qll index 3e4642411f0c..bf79523f50f9 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CleartextStorageQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CleartextStorageQuery.qll @@ -4,6 +4,7 @@ import csharp private import semmle.code.csharp.security.dataflow.flowsources.Remote +private import semmle.code.csharp.dataflow.internal.ExternalFlow private import semmle.code.csharp.frameworks.system.Web private import semmle.code.csharp.security.SensitiveActions private import semmle.code.csharp.security.dataflow.flowsinks.ExternalLocationSink @@ -62,3 +63,5 @@ class ProtectSanitizer extends Sanitizer { * An external location sink. */ class ExternalSink extends Sink instanceof ExternalLocationSink { } + +private class ExternalSanitizer extends Sanitizer instanceof ExternalLocationSanitizer { } diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CodeInjectionQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CodeInjectionQuery.qll index 20d5bbe10cab..f567e3cbc021 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CodeInjectionQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CodeInjectionQuery.qll @@ -95,7 +95,12 @@ class RoslynCSharpScriptSink extends Sink { } } -/** Code injection sinks defined through CSV models. */ +/** A code injection sink defined through Models as Data. */ private class ExternalCodeInjectionExprSink extends Sink { ExternalCodeInjectionExprSink() { sinkNode(this, "code-injection") } } + +/** A sanitizer for code injection defined through Models as Data. */ +private class ExternalCodeInjectionSanitizer extends Sanitizer { + ExternalCodeInjectionSanitizer() { barrierNode(this, "code-injection") } +} diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CommandInjectionQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CommandInjectionQuery.qll index 9528676af8e6..b8c37d602b94 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CommandInjectionQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CommandInjectionQuery.qll @@ -61,11 +61,16 @@ deprecated class RemoteSource extends DataFlow::Node instanceof RemoteFlowSource /** A source supported by the current threat model. */ class ThreatModelSource extends Source instanceof ActiveThreatModelSource { } -/** Command Injection sinks defined through Models as Data. */ +/** A Command Injection sink defined through Models as Data. */ private class ExternalCommandInjectionExprSink extends Sink { ExternalCommandInjectionExprSink() { sinkNode(this, "command-injection") } } +/** A sanitizer for command injection defined through Models as Data. */ +private class ExternalCommandInjectionSanitizer extends Sanitizer { + ExternalCommandInjectionSanitizer() { barrierNode(this, "command-injection") } +} + /** * A sink in `System.Diagnostic.Process` or its related classes. */ diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformationQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformationQuery.qll index 85782fe49369..b5b95902d8ab 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformationQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformationQuery.qll @@ -46,3 +46,5 @@ private class PrivateDataSource extends Source { } private class ExternalLocation extends Sink instanceof ExternalLocationSink { } + +private class ExternalSanitizer extends Sanitizer instanceof ExternalLocationSanitizer { } diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/LDAPInjectionQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/LDAPInjectionQuery.qll index c2e25f0e522d..06bfac6c5458 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/LDAPInjectionQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/LDAPInjectionQuery.qll @@ -64,11 +64,16 @@ deprecated class RemoteSource extends DataFlow::Node instanceof RemoteFlowSource /** A source supported by the current threat model. */ class ThreatModelSource extends Source instanceof ActiveThreatModelSource { } -/** LDAP sinks defined through Models as Data. */ +/** An LDAP sink defined through Models as Data. */ private class ExternalLdapExprSink extends Sink { ExternalLdapExprSink() { sinkNode(this, "ldap-injection") } } +/** A sanitizer for LDAP injection defined through Models as Data. */ +private class ExternalLdapInjectionSanitizer extends Sanitizer { + ExternalLdapInjectionSanitizer() { barrierNode(this, "ldap-injection") } +} + /** * An argument that sets the `Path` property of a `DirectoryEntry` object that is a sink for LDAP * injection. diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/LogForgingQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/LogForgingQuery.qll index e06e728514de..22023ebc4090 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/LogForgingQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/LogForgingQuery.qll @@ -61,11 +61,16 @@ private class LogForgingLogMessageSink extends Sink, LogMessageSink { } */ private class LogForgingTraceMessageSink extends Sink, TraceMessageSink { } -/** Log Forging sinks defined through Models as Data. */ +/** A Log Forging sink defined through Models as Data. */ private class ExternalLoggingExprSink extends Sink { ExternalLoggingExprSink() { sinkNode(this, "log-injection") } } +/** A sanitizer for log forging defined through Models as Data. */ +private class ExternalLogForgingSanitizer extends Sanitizer { + ExternalLogForgingSanitizer() { barrierNode(this, "log-injection") } +} + /** * A call to String replace or remove that is considered to sanitize replaced string. */ diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/SqlInjectionQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/SqlInjectionQuery.qll index 510b03811432..addc19321776 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/SqlInjectionQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/SqlInjectionQuery.qll @@ -74,11 +74,16 @@ class SqlInjectionExprSink extends Sink { SqlInjectionExprSink() { exists(SqlExpr s | this.getExpr() = s.getSql()) } } -/** SQL sinks defined through CSV models. */ +/** An SQL sink defined through CSV models. */ private class ExternalSqlInjectionExprSink extends Sink { ExternalSqlInjectionExprSink() { sinkNode(this, "sql-injection") } } +/** A sanitizer for SQL injection defined through Models as Data. */ +private class ExternalSqlInjectionSanitizer extends Sanitizer { + ExternalSqlInjectionSanitizer() { barrierNode(this, "sql-injection") } +} + private class SimpleTypeSanitizer extends Sanitizer, SimpleTypeSanitizedExpr { } private class GuidSanitizer extends Sanitizer, GuidSanitizedExpr { } diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll index 15ba99aedf0d..bad6c990fa7b 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll @@ -56,11 +56,16 @@ deprecated class RemoteSource extends DataFlow::Node instanceof RemoteFlowSource /** A source supported by the current threat model. */ class ThreatModelSource extends Source instanceof ActiveThreatModelSource { } -/** URL Redirection sinks defined through Models as Data. */ +/** A URL Redirection sink defined through Models as Data. */ private class ExternalUrlRedirectExprSink extends Sink { ExternalUrlRedirectExprSink() { sinkNode(this, "url-redirection") } } +/** A sanitizer for URL redirection defined through Models as Data. */ +private class ExternalUrlRedirectSanitizer extends Sanitizer { + ExternalUrlRedirectSanitizer() { barrierNode(this, "url-redirection") } +} + /** * A URL argument to a call to `HttpResponse.Redirect()` or `Controller.Redirect()`, that is a * sink for URL redirects. @@ -160,27 +165,6 @@ class ContainsUrlSanitizer extends Sanitizer { } } -/** - * A check that the URL is relative, and therefore safe for URL redirects. - */ -private predicate isRelativeUrlSanitizer(Guard guard, Expr e, GuardValue v) { - guard = - any(PropertyAccess access | - access.getProperty().hasFullyQualifiedName("System", "Uri", "IsAbsoluteUri") and - e = access.getQualifier() and - v.asBooleanValue() = false - ) -} - -/** - * A check that the URL is relative, and therefore safe for URL redirects. - */ -class RelativeUrlSanitizer extends Sanitizer { - RelativeUrlSanitizer() { - this = DataFlow::BarrierGuard::getABarrierNode() - } -} - /** * A comparison on the `Host` property of a url, that is a sanitizer for URL redirects. * E.g. `url.Host == "example.org"` @@ -205,16 +189,6 @@ class HostComparisonSanitizer extends Sanitizer { } } -/** - * A call to the getter of the RawUrl property, whose value is considered to be safe for URL - * redirects. - */ -class RawUrlSanitizer extends Sanitizer { - RawUrlSanitizer() { - this.getExpr() = any(SystemWebHttpRequestClass r).getRawUrlProperty().getGetter().getACall() - } -} - /** * A string concatenation expression, where the left hand side contains the character "?". * diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/XSSQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/XSSQuery.qll index 2d687b51d67a..b084905ddade 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/XSSQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/XSSQuery.qll @@ -7,6 +7,7 @@ import csharp private import XSSSinks private import semmle.code.csharp.security.Sanitizers private import semmle.code.csharp.security.dataflow.flowsources.FlowSources +private import semmle.code.csharp.dataflow.internal.ExternalFlow /** * Holds if there is tainted flow from `source` to `sink` that may lead to a @@ -169,6 +170,11 @@ private class SimpleTypeSanitizer extends Sanitizer, SimpleTypeSanitizedExpr { } private class GuidSanitizer extends Sanitizer, GuidSanitizedExpr { } +/** A sanitizer for XSS defined through Models as Data. */ +private class ExternalXssSanitizer extends Sanitizer { + ExternalXssSanitizer() { barrierNode(this, ["html-injection", "js-injection"]) } +} + /** A call to an HTML encoder. */ private class HtmlEncodeSanitizer extends Sanitizer { HtmlEncodeSanitizer() { this.getExpr() instanceof HtmlSanitizedExpr } 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 index 3bcfdde669a4..4ee02416961d 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll @@ -126,3 +126,11 @@ class LocalFileOutputSink extends ExternalLocationSink { ) } } + +/** + * A sanitizer for writing data to locations that are external to the + * application, defined through Models as Data. + */ +class ExternalLocationSanitizer extends DataFlow::Node { + ExternalLocationSanitizer() { barrierNode(this, "file-content-store") } +}