diff --git a/change-notes/1.24/analysis-csharp.md b/change-notes/1.24/analysis-csharp.md index 02f31b12533..a745e985eae 100644 --- a/change-notes/1.24/analysis-csharp.md +++ b/change-notes/1.24/analysis-csharp.md @@ -22,6 +22,8 @@ The following changes in version 1.24 affect C# analysis in all applications. | Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | More results | Results are reported from parameters with a default value of `null`. | | Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the value assigned is an (implicitly or explicitly) cast default-like value. For example, `var s = (string)null` and `string s = default`. | | XPath injection (`cs/xml/xpath-injection`) | More results | The query now recognizes calls to methods on `System.Xml.XPath.XPathNavigator` objects. | +| Information exposure through transmitted data (`cs/sensitive-data-transmission`) | More results | The query now recognizes writes to cookies and writes to ASP.NET (`Inner`)`Text` properties as additional sinks. | +| Information exposure through an exception (`cs/information-exposure-through-exception`) | More results | The query now recognizes writes to cookies, writes to ASP.NET (`Inner`)`Text` properties, and email contents as additional sinks. | ## Removal of old queries @@ -42,5 +44,6 @@ The following changes in version 1.24 affect C# analysis in all applications. * [Code contracts](https://docs.microsoft.com/en-us/dotnet/framework/debug-trace-profile/code-contracts) are now recognized, and are treated like any other assertion methods. * Expression nullability flow state is given by the predicates `Expr.hasNotNullFlowState()` and `Expr.hasMaybeNullFlowState()`. * `stackalloc` array creations are now represented by the QL class `Stackalloc`. Previously they were represented by the class `ArrayCreation`. +* A new class `RemoteFlowSink` has been added to model sinks where data might be exposed to external users. Examples include web page output, e-mails, and cookies. ## Changes to autobuilder diff --git a/csharp/ql/src/Security Features/CWE-091/XMLInjection.ql b/csharp/ql/src/Security Features/CWE-091/XMLInjection.ql index 6b9acf69e3c..ac9a3d90dbd 100644 --- a/csharp/ql/src/Security Features/CWE-091/XMLInjection.ql +++ b/csharp/ql/src/Security Features/CWE-091/XMLInjection.ql @@ -11,7 +11,7 @@ */ import csharp -import semmle.code.csharp.dataflow.flowsources.Remote +import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.Xml /** diff --git a/csharp/ql/src/Security Features/CWE-114/AssemblyPathInjection.ql b/csharp/ql/src/Security Features/CWE-114/AssemblyPathInjection.ql index 2b87d61f193..0aab5e3181a 100644 --- a/csharp/ql/src/Security Features/CWE-114/AssemblyPathInjection.ql +++ b/csharp/ql/src/Security Features/CWE-114/AssemblyPathInjection.ql @@ -12,7 +12,7 @@ */ import csharp -import semmle.code.csharp.dataflow.flowsources.Remote +import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.commons.Util /** diff --git a/csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql b/csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql index 1b42ea7399d..3b9d6af7d14 100644 --- a/csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql +++ b/csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql @@ -11,8 +11,8 @@ */ import csharp -import semmle.code.csharp.dataflow.flowsources.Remote -import semmle.code.csharp.dataflow.flowsources.Local +import semmle.code.csharp.security.dataflow.flowsources.Remote +import semmle.code.csharp.security.dataflow.flowsources.Local import semmle.code.csharp.dataflow.TaintTracking import semmle.code.csharp.frameworks.Format import DataFlow::PathGraph diff --git a/csharp/ql/src/Security Features/CWE-201/ExposureInTransmittedData.ql b/csharp/ql/src/Security Features/CWE-201/ExposureInTransmittedData.ql index 4419a4f1312..b8b18c6b56d 100644 --- a/csharp/ql/src/Security Features/CWE-201/ExposureInTransmittedData.ql +++ b/csharp/ql/src/Security Features/CWE-201/ExposureInTransmittedData.ql @@ -11,8 +11,7 @@ import csharp import semmle.code.csharp.security.SensitiveActions -import semmle.code.csharp.security.dataflow.XSS -import semmle.code.csharp.security.dataflow.Email +import semmle.code.csharp.security.dataflow.flowsinks.Remote import semmle.code.csharp.frameworks.system.data.Common import semmle.code.csharp.frameworks.System import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph @@ -42,11 +41,7 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration { ) } - override predicate isSink(DataFlow::Node sink) { - sink instanceof XSS::Sink - or - sink instanceof Email::Sink - } + override predicate isSink(DataFlow::Node sink) { sink instanceof RemoteFlowSink } } from TaintTrackingConfiguration configuration, DataFlow::PathNode source, DataFlow::PathNode sink diff --git a/csharp/ql/src/Security Features/CWE-209/ExceptionInformationExposure.ql b/csharp/ql/src/Security Features/CWE-209/ExceptionInformationExposure.ql index 8cebb2601c4..d9db652c8d8 100644 --- a/csharp/ql/src/Security Features/CWE-209/ExceptionInformationExposure.ql +++ b/csharp/ql/src/Security Features/CWE-209/ExceptionInformationExposure.ql @@ -14,7 +14,7 @@ import csharp import semmle.code.csharp.frameworks.System -import semmle.code.csharp.security.dataflow.XSS +import semmle.code.csharp.security.dataflow.flowsinks.Remote import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph /** @@ -46,7 +46,7 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration { ) } - override predicate isSink(DataFlow::Node sink) { sink instanceof XSS::Sink } + override predicate isSink(DataFlow::Node sink) { sink instanceof RemoteFlowSink } override predicate isSanitizer(DataFlow::Node sanitizer) { // Do not flow through Message diff --git a/csharp/ql/src/Security Features/CWE-838/InappropriateEncoding.ql b/csharp/ql/src/Security Features/CWE-838/InappropriateEncoding.ql index 88a5c3da4ed..88a0b970a7a 100644 --- a/csharp/ql/src/Security Features/CWE-838/InappropriateEncoding.ql +++ b/csharp/ql/src/Security Features/CWE-838/InappropriateEncoding.ql @@ -16,7 +16,7 @@ import semmle.code.csharp.frameworks.system.Net import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.frameworks.system.web.UI import semmle.code.csharp.security.dataflow.SqlInjection -import semmle.code.csharp.security.dataflow.XSS +import semmle.code.csharp.security.dataflow.flowsinks.Html import semmle.code.csharp.security.dataflow.UrlRedirect import semmle.code.csharp.security.Sanitizers import semmle.code.csharp.dataflow.DataFlow2::DataFlow2 @@ -114,7 +114,7 @@ module EncodingConfigurations { override string getKind() { result = "HTML expression" } - override predicate requiresEncoding(Node n) { n instanceof XSS::HtmlSink } + override predicate requiresEncoding(Node n) { n instanceof HtmlSink } override predicate isPossibleEncodedValue(Expr e) { e instanceof HtmlSanitizedExpr } } diff --git a/csharp/ql/src/Security Features/InsecureRandomness.ql b/csharp/ql/src/Security Features/InsecureRandomness.ql index bd16dda8816..ef1665819f7 100644 --- a/csharp/ql/src/Security Features/InsecureRandomness.ql +++ b/csharp/ql/src/Security Features/InsecureRandomness.ql @@ -16,7 +16,7 @@ import semmle.code.csharp.frameworks.Test import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph module Random { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.security.SensitiveActions /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/PublicCallableParameter.qll b/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/PublicCallableParameter.qll index 07dc38d320d..cf69f54717a 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/PublicCallableParameter.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/PublicCallableParameter.qll @@ -1,9 +1,10 @@ /** + * DEPRECATED. + * * Provides classes representing data flow sources for parameters of public callables. */ import csharp -private import semmle.code.csharp.frameworks.WCF /** * A parameter of a public callable, for example `p` in diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll b/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll index f8240583108..07a23b36c26 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll @@ -1,218 +1,7 @@ /** - * Provides classes representing data flow sources for remote user input. + * DEPRECATED. + * + * Use `semmle.code.csharp.security.dataflow.flowsources.Remote` instead. */ -import csharp -private import semmle.code.csharp.frameworks.system.Net -private import semmle.code.csharp.frameworks.system.Web -private import semmle.code.csharp.frameworks.system.web.Http -private import semmle.code.csharp.frameworks.system.web.Mvc -private import semmle.code.csharp.frameworks.system.web.Services -private import semmle.code.csharp.frameworks.system.web.ui.WebControls -private import semmle.code.csharp.frameworks.WCF -private import semmle.code.csharp.frameworks.microsoft.Owin -private import semmle.code.csharp.frameworks.microsoft.AspNetCore - -/** A data flow source of remote user input. */ -abstract class RemoteFlowSource extends DataFlow::Node { - /** Gets a string that describes the type of this remote flow source. */ - abstract string getSourceType(); -} - -/** A data flow source of remote user input (ASP.NET). */ -abstract class AspNetRemoteFlowSource extends RemoteFlowSource { } - -/** A member containing an ASP.NET query string. */ -class AspNetQueryStringMember extends Member { - AspNetQueryStringMember() { - exists(RefType t | - t instanceof SystemWebHttpRequestClass or - t instanceof SystemNetHttpListenerRequestClass or - t instanceof SystemWebHttpRequestBaseClass - | - this = t.getProperty(getHttpRequestFlowPropertyNames()) or - this.(Field).getType() = t or - this.(Property).getType() = t or - this.(Callable).getReturnType() = t - ) - } -} - -/** - * Gets the names of the properties in `HttpRequest` classes that should propagate taint out of the - * request. - */ -private string getHttpRequestFlowPropertyNames() { - result = "QueryString" or - result = "Headers" or - result = "RawUrl" or - result = "Url" or - result = "Cookies" or - result = "Form" or - result = "Params" or - result = "Path" or - result = "PathInfo" -} - -/** A data flow source of remote user input (ASP.NET query string). */ -class AspNetQueryStringRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode { - AspNetQueryStringRemoteFlowSource() { - exists(RefType t | - t instanceof SystemWebHttpRequestClass or - t instanceof SystemNetHttpListenerRequestClass or - t instanceof SystemWebHttpRequestBaseClass - | - // A request object can be indexed, so taint the object as well - this.getExpr().getType() = t - ) - or - this.getExpr() = any(AspNetQueryStringMember m).getAnAccess() - } - - override string getSourceType() { result = "ASP.NET query string" } -} - -/** A data flow source of remote user input (ASP.NET unvalidated request data). */ -class AspNetUnvalidatedQueryStringRemoteFlowSource extends AspNetRemoteFlowSource, - DataFlow::ExprNode { - AspNetUnvalidatedQueryStringRemoteFlowSource() { - this.getExpr() = any(SystemWebUnvalidatedRequestValues c).getAProperty().getGetter().getACall() or - this.getExpr() = - any(SystemWebUnvalidatedRequestValuesBase c).getAProperty().getGetter().getACall() - } - - override string getSourceType() { result = "ASP.NET unvalidated request data" } -} - -/** A data flow source of remote user input (ASP.NET user input). */ -class AspNetUserInputRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode { - AspNetUserInputRemoteFlowSource() { getType() instanceof SystemWebUIWebControlsTextBoxClass } - - override string getSourceType() { result = "ASP.NET user input" } -} - -/** A data flow source of remote user input (WCF based web service). */ -class WcfRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { - WcfRemoteFlowSource() { exists(OperationMethod om | om.getAParameter() = this.getParameter()) } - - override string getSourceType() { result = "web service input" } -} - -/** A data flow source of remote user input (ASP.NET web service). */ -class AspNetServiceRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { - AspNetServiceRemoteFlowSource() { - exists(Method m | - m.getAParameter() = this.getParameter() and - m.getAnAttribute().getType() instanceof SystemWebServicesWebMethodAttributeClass - ) - } - - override string getSourceType() { result = "ASP.NET web service input" } -} - -/** A data flow source of remote user input (ASP.NET request message). */ -class SystemNetHttpRequestMessageRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode { - SystemNetHttpRequestMessageRemoteFlowSource() { - getType() instanceof SystemWebHttpRequestMessageClass - } - - override string getSourceType() { result = "ASP.NET request message" } -} - -/** - * A data flow source of remote user input (Microsoft Owin, a query, request, - * or path string). - */ -class MicrosoftOwinStringFlowSource extends RemoteFlowSource, DataFlow::ExprNode { - MicrosoftOwinStringFlowSource() { - this.getExpr() = any(MicrosoftOwinString owinString).getValueProperty().getGetter().getACall() - } - - override string getSourceType() { result = "Microsoft Owin request or query string" } -} - -/** A data flow source of remote user input (`Microsoft Owin IOwinRequest`). */ -class MicrosoftOwinRequestRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode { - MicrosoftOwinRequestRemoteFlowSource() { - exists(Property p, MicrosoftOwinIOwinRequestClass owinRequest | - this.getExpr() = p.getGetter().getACall() - | - p = owinRequest.getAcceptProperty() or - p = owinRequest.getBodyProperty() or - p = owinRequest.getCacheControlProperty() or - p = owinRequest.getContentTypeProperty() or - p = owinRequest.getContextProperty() or - p = owinRequest.getCookiesProperty() or - p = owinRequest.getHeadersProperty() or - p = owinRequest.getHostProperty() or - p = owinRequest.getMediaTypeProperty() or - p = owinRequest.getMethodProperty() or - p = owinRequest.getPathProperty() or - p = owinRequest.getPathBaseProperty() or - p = owinRequest.getQueryProperty() or - p = owinRequest.getQueryStringProperty() or - p = owinRequest.getRemoteIpAddressProperty() or - p = owinRequest.getSchemeProperty() or - p = owinRequest.getURIProperty() - ) - } - - override string getSourceType() { result = "Microsoft Owin request" } -} - -/** A parameter to an Mvc controller action method, viewed as a source of remote user input. */ -class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { - ActionMethodParameter() { - exists(Parameter p | - p = this.getParameter() and - p.fromSource() - | - p = any(Controller c).getAnActionMethod().getAParameter() or - p = any(ApiController c).getAnActionMethod().getAParameter() - ) - } - - override string getSourceType() { result = "ASP.NET MVC action method parameter" } -} - -/** A data flow source of remote user input (ASP.NET Core). */ -abstract class AspNetCoreRemoteFlowSource extends RemoteFlowSource { } - -/** A data flow source of remote user input (ASP.NET query collection). */ -class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFlow::ExprNode { - AspNetCoreQueryRemoteFlowSource() { - exists(ValueOrRefType t | - t instanceof MicrosoftAspNetCoreHttpHttpRequest or - t instanceof MicrosoftAspNetCoreHttpQueryCollection or - t instanceof MicrosoftAspNetCoreHttpQueryString - | - this.getExpr().(Call).getTarget().getDeclaringType() = t or - this.asExpr().(Access).getTarget().getDeclaringType() = t - ) - or - exists(Call c | - c - .getTarget() - .getDeclaringType() - .hasQualifiedName("Microsoft.AspNetCore.Http", "IQueryCollection") and - c.getTarget().getName() = "TryGetValue" and - this.asExpr() = c.getArgumentForName("value") - ) - } - - override string getSourceType() { result = "ASP.NET Core query string" } -} - -/** A parameter to a `Mvc` controller action method, viewed as a source of remote user input. */ -class AspNetCoreActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { - AspNetCoreActionMethodParameter() { - exists(Parameter p | - p = this.getParameter() and - p.fromSource() - | - p = any(MicrosoftAspNetCoreMvcController c).getAnActionMethod().getAParameter() - ) - } - - override string getSourceType() { result = "ASP.NET Core MVC action method parameter" } -} +import semmle.code.csharp.security.dataflow.flowsources.Remote diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/CleartextStorage.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/CleartextStorage.qll index 8eff8c88e3f..a0ce6d9d1ae 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/CleartextStorage.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/CleartextStorage.qll @@ -5,10 +5,10 @@ import csharp module CleartextStorage { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.security.SensitiveActions - import semmle.code.csharp.security.sinks.ExternalLocationSink + import semmle.code.csharp.security.dataflow.flowsinks.ExternalLocationSink /** * A data flow source for cleartext storage of sensitive information. diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/CodeInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/CodeInjection.qll index 3c168463d9d..d58d851b7c8 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/CodeInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/CodeInjection.qll @@ -5,8 +5,8 @@ import csharp module CodeInjection { - import semmle.code.csharp.dataflow.flowsources.Remote - import semmle.code.csharp.dataflow.flowsources.Local + import semmle.code.csharp.security.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Local import semmle.code.csharp.frameworks.system.codedom.Compiler import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll index 01d6c01fd2b..7d2a49784e1 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll @@ -5,7 +5,7 @@ import csharp module CommandInjection { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.Diagnostics import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll index 18c725d273e..0694b27f985 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll @@ -8,7 +8,7 @@ import csharp module UserControlledBypassOfSensitiveMethod { import semmle.code.csharp.controlflow.Guards import semmle.code.csharp.controlflow.BasicBlocks - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.System import semmle.code.csharp.frameworks.system.Net import semmle.code.csharp.security.SensitiveActions diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformation.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformation.qll index 45d5794dca6..19866738341 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformation.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformation.qll @@ -5,8 +5,8 @@ import csharp module ExposureOfPrivateInformation { - import semmle.code.csharp.dataflow.flowsources.Remote - import semmle.code.csharp.security.sinks.ExternalLocationSink + import semmle.code.csharp.security.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsinks.ExternalLocationSink import semmle.code.csharp.security.PrivateData /** diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/LDAPInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/LDAPInjection.qll index 329e001236f..50681f9721c 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/LDAPInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/LDAPInjection.qll @@ -6,7 +6,7 @@ import csharp module LDAPInjection { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.DirectoryServices import semmle.code.csharp.frameworks.system.directoryservices.Protocols import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/LogForging.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/LogForging.qll index 3460362cf99..400b1ac8763 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/LogForging.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/LogForging.qll @@ -5,11 +5,11 @@ import csharp module LogForging { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.System import semmle.code.csharp.frameworks.system.text.RegularExpressions import semmle.code.csharp.security.Sanitizers - import semmle.code.csharp.security.sinks.ExternalLocationSink + import semmle.code.csharp.security.dataflow.flowsinks.ExternalLocationSink /** * A data flow source for untrusted user input used in log entries. diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll index 28b8350a200..ba5ee7147d3 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll @@ -6,7 +6,7 @@ import csharp module MissingXMLValidation { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.Xml import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/ReDoS.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/ReDoS.qll index bb40b62a0be..28c5c79e0af 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/ReDoS.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/ReDoS.qll @@ -7,7 +7,7 @@ import csharp module ReDoS { private import semmle.code.csharp.dataflow.DataFlow2 - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.text.RegularExpressions import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/RegexInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/RegexInjection.qll index 51f8605c1af..2edfbc4ab7c 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/RegexInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/RegexInjection.qll @@ -6,7 +6,7 @@ import csharp module RegexInjection { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.text.RegularExpressions import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/ResourceInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/ResourceInjection.qll index 6ae49c6f7d5..236fe62aa2c 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/ResourceInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/ResourceInjection.qll @@ -5,8 +5,8 @@ import csharp module ResourceInjection { - import semmle.code.csharp.dataflow.flowsources.Remote - import semmle.code.csharp.dataflow.flowsources.Local + import semmle.code.csharp.security.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Local import semmle.code.csharp.frameworks.system.Data import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/SqlInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/SqlInjection.qll index cb393e00179..21c2628aa62 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/SqlInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/SqlInjection.qll @@ -5,8 +5,8 @@ import csharp module SqlInjection { - import semmle.code.csharp.dataflow.flowsources.Remote - import semmle.code.csharp.dataflow.flowsources.Local + import semmle.code.csharp.security.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Local import semmle.code.csharp.frameworks.Sql import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll index 78a0ca2c3bf..df59f0a4793 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll @@ -7,7 +7,7 @@ import csharp module TaintedPath { import semmle.code.csharp.controlflow.Guards - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.IO import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/UnsafeDeserialization.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/UnsafeDeserialization.qll index 57b3a78485b..0943774d8cd 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/UnsafeDeserialization.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/UnsafeDeserialization.qll @@ -6,7 +6,7 @@ import csharp module UnsafeDeserialization { - private import semmle.code.csharp.dataflow.flowsources.Remote + private import semmle.code.csharp.security.dataflow.flowsources.Remote private import semmle.code.csharp.serialization.Deserializers /** diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll index ff501a1c096..2008e62c60d 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll @@ -5,7 +5,7 @@ import csharp module UrlRedirect { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.controlflow.Guards import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.frameworks.system.web.Mvc diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/XMLEntityInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/XMLEntityInjection.qll index 67c5cb552f5..425fd6b4019 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/XMLEntityInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/XMLEntityInjection.qll @@ -5,7 +5,7 @@ import csharp module XMLEntityInjection { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.System import semmle.code.csharp.frameworks.system.text.RegularExpressions import semmle.code.csharp.security.xml.InsecureXML diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/XPathInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/XPathInjection.qll index e2ef4797dfe..7c84561cf44 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/XPathInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/XPathInjection.qll @@ -5,7 +5,7 @@ import csharp module XPathInjection { - import semmle.code.csharp.dataflow.flowsources.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote import semmle.code.csharp.frameworks.system.xml.XPath import semmle.code.csharp.frameworks.system.Xml import semmle.code.csharp.security.Sanitizers diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll index 513b0b89f00..635c59363f5 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll @@ -6,17 +6,14 @@ import csharp module XSS { - import semmle.code.csharp.dataflow.flowsources.Remote - import semmle.code.csharp.frameworks.microsoft.AspNetCore + import semmle.code.asp.AspNet import semmle.code.csharp.frameworks.system.Net import semmle.code.csharp.frameworks.system.Web - import semmle.code.csharp.frameworks.system.web.Mvc - import semmle.code.csharp.frameworks.system.web.WebPages import semmle.code.csharp.frameworks.system.web.UI - import semmle.code.csharp.frameworks.system.web.ui.WebControls - import semmle.code.csharp.frameworks.system.windows.Forms import semmle.code.csharp.security.Sanitizers - import semmle.code.asp.AspNet + import semmle.code.csharp.security.dataflow.flowsinks.Html + import semmle.code.csharp.security.dataflow.flowsinks.Remote + import semmle.code.csharp.security.dataflow.flowsources.Remote /** * Holds if there is tainted flow from `source` to `sink` that may lead to a @@ -112,8 +109,11 @@ module XSS { /** * A data flow sink for cross-site scripting (XSS) vulnerabilities. + * + * Any XSS sink is also a remote flow sink, so this class contributes + * to the abstract class `RemoteFlowSink`. */ - abstract class Sink extends DataFlow::ExprNode { + abstract class Sink extends DataFlow::ExprNode, RemoteFlowSink { string explanation() { none() } } @@ -166,78 +166,21 @@ module XSS { UrlEncodeSanitizer() { this.getExpr() instanceof UrlSanitizedExpr } } - /** A sink where the value of the expression may be rendered as HTML. */ - abstract class HtmlSink extends DataFlow::Node { } + private class HtmlSinkSink extends Sink { + HtmlSinkSink() { this instanceof HtmlSink } - /** - * An expression that is used as an argument to an XSS sink method on - * `HttpResponse`. - */ - private class HttpResponseSink extends Sink, HtmlSink { - HttpResponseSink() { - exists(Method m, SystemWebHttpResponseClass responseClass | - m = responseClass.getAWriteMethod() or - m = responseClass.getAWriteFileMethod() or - m = responseClass.getATransmitFileMethod() or - m = responseClass.getABinaryWriteMethod() - | - // Calls to these methods, or overrides of them - this.getExpr() = m.getAnOverrider*().getParameter(0).getAnAssignedArgument() - ) - } - } - - /** - * An expression that is used as an argument to an XSS sink method on - * `HtmlTextWriter`. - */ - private class HtmlTextWriterSink extends Sink, HtmlSink { - HtmlTextWriterSink() { - exists(SystemWebUIHtmlTextWriterClass writeClass, Method m, Call c, int paramPos | - paramPos = 0 and - ( - m = writeClass.getAWriteMethod() or - m = writeClass.getAWriteLineMethod() or - m = writeClass.getAWriteLineNoTabsMethod() or - m = writeClass.getAWriteBeginTagMethod() or - m = writeClass.getAWriteAttributeMethod() - ) - or - // The second parameter to the `WriteAttribute` method is the attribute value, which we - // should only consider as tainted if the call does not ask for the attribute value to be - // encoded using the final parameter. - m = writeClass.getAWriteAttributeMethod() and - paramPos = 1 and - not c.getArgumentForParameter(m.getParameter(2)).(BoolLiteral).getBoolValue() = true - | - c = m.getACall() and - this.getExpr() = c.getArgumentForParameter(m.getParameter(paramPos)) - ) - } - } - - /** - * An expression that is used as an argument to an XSS sink method on - * `AttributeCollection`. - */ - private class AttributeCollectionSink extends Sink, HtmlSink { - AttributeCollectionSink() { - exists(SystemWebUIAttributeCollectionClass ac, Parameter p | - p = ac.getAddMethod().getParameter(1) or - p = ac.getItemProperty().getSetter().getParameter(0) - | - this.getExpr() = p.getAnAssignedArgument() - ) - } - } - - /** - * An expression that is used as the second argument `HtmlElement.SetAttribute`. - */ - private class SetAttributeSink extends Sink, HtmlSink { - SetAttributeSink() { - this.getExpr() = - any(SystemWindowsFormsHtmlElement c).getSetAttributeMethod().getACall().getArgument(1) + override string explanation() { + this instanceof WebPageWriteLiteralSink and + result = "System.Web.WebPages.WebPage.WriteLiteral() method" + or + this instanceof WebPageWriteLiteralToSink and + result = "System.Web.WebPages.WebPage.WriteLiteralTo() method" + or + this instanceof MicrosoftAspNetCoreMvcHtmlHelperRawSink and + result = "Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method" + or + this instanceof MicrosoftAspNetRazorPageWriteLiteralSink and + result = "Microsoft.AspNetCore.Mvc.Razor.RazorPageBase.WriteLiteral() method" } } @@ -285,31 +228,6 @@ module XSS { } } - /** - * An expression that is used as an argument to an XSS sink setter, on - * a class within the `System.Web.UI` namespace. - */ - private class SystemWebSetterHtmlSink extends Sink, HtmlSink { - SystemWebSetterHtmlSink() { - exists(Property p, string name, ValueOrRefType declaringType | - declaringType = p.getDeclaringType() and - any(SystemWebUINamespace n).getAChildNamespace*() = declaringType.getNamespace() and - this.getExpr() = p.getSetter().getParameter(0).getAnAssignedArgument() and - p.hasName(name) - | - name = "Caption" and - (declaringType.hasName("Calendar") or declaringType.hasName("Table")) - or - name = "InnerHtml" - ) - or - exists(SystemWebUIWebControlsLabelClass c | - // Unlike `Text` properties of other web controls, `Label.Text` is not automatically HTML encoded - this.getExpr() = c.getTextProperty().getSetter().getParameter(0).getAnAssignedArgument() - ) - } - } - /** * An expression that is used as an argument to an XSS sink setter, on * a class within the `System.Web.UI` namespace. @@ -345,16 +263,6 @@ module XSS { } } - /** - * An expression that is used as an argument to `HtmlHelper.Raw`, typically in - * a `.cshtml` file. - */ - private class SystemWebMvcHtmlHelperRawSink extends Sink, HtmlSink { - SystemWebMvcHtmlHelperRawSink() { - this.getExpr() = any(SystemWebMvcHtmlHelperClass h).getRawMethod().getACall().getAnArgument() - } - } - /** * Gets a member which is accessed by the given `AspInlineCode`. * The code body must consist only of an access to the member, possibly with qualified @@ -493,31 +401,6 @@ module XSS { } } - /** An expression that is returned from a `ToHtmlString` method. */ - private class ToHtmlString extends Sink, HtmlSink { - ToHtmlString() { - exists(Method toHtmlString | - toHtmlString = - any(SystemWebIHtmlString i).getToHtmlStringMethod().getAnUltimateImplementor() and - toHtmlString.canReturn(this.getExpr()) - ) - } - } - - /** - * An expression passed to the constructor of an `HtmlString` or a `MvcHtmlString`. - */ - private class HtmlString extends Sink, HtmlSink { - HtmlString() { - exists(Class c | - c = any(SystemWebMvcMvcHtmlString m) or - c = any(SystemWebHtmlString m) - | - this.getExpr() = c.getAConstructor().getACall().getAnArgument() - ) - } - } - /** * An expression passed as the `content` argument to the constructor of `StringContent`. */ @@ -529,75 +412,6 @@ module XSS { ).getArgumentForName("content") } } - - /** - * An expression that is used as an argument to `Page.WriteLiteral`, typically in - * a `.cshtml` file. - */ - class WebPageWriteLiteralSink extends Sink, HtmlSink { - WebPageWriteLiteralSink() { - this.getExpr() = any(WebPageClass h).getWriteLiteralMethod().getACall().getAnArgument() - } - - override string explanation() { result = "System.Web.WebPages.WebPage.WriteLiteral() method" } - } - - /** - * An expression that is used as an argument to `Page.WriteLiteralTo`, typically in - * a `.cshtml` file. - */ - class WebPageWriteLiteralToSink extends Sink, HtmlSink { - WebPageWriteLiteralToSink() { - this.getExpr() = any(WebPageClass h).getWriteLiteralToMethod().getACall().getAnArgument() - } - - override string explanation() { result = "System.Web.WebPages.WebPage.WriteLiteralTo() method" } - } - - abstract class AspNetCoreSink extends Sink, HtmlSink { } - - /** - * An expression that is used as an argument to `HtmlHelper.Raw`, typically in - * a `.cshtml` file. - */ - class MicrosoftAspNetCoreMvcHtmlHelperRawSink extends AspNetCoreSink { - MicrosoftAspNetCoreMvcHtmlHelperRawSink() { - this.getExpr() = - any(MicrosoftAspNetCoreMvcHtmlHelperClass h).getRawMethod().getACall().getAnArgument() - } - - override string explanation() { - result = "Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method" - } - } - - /** - * An expression that is used as an argument to `Page.WriteLiteral` in ASP.NET 6.0 razor page, typically in - * a `.cshtml` file. - */ - class MicrosoftAspNetRazorPageWriteLiteralSink extends AspNetCoreSink { - MicrosoftAspNetRazorPageWriteLiteralSink() { - this.getExpr() = - any(MicrosoftAspNetCoreMvcRazorPageBase h) - .getWriteLiteralMethod() - .getACall() - .getAnArgument() - } - - override string explanation() { - result = "Microsoft.AspNetCore.Mvc.Razor.RazorPageBase.WriteLiteral() method" - } - } - - /** `HtmlString` that may be rendered as is need to have sanitized value. */ - class MicrosoftAspNetHtmlStringSink extends AspNetCoreSink { - MicrosoftAspNetHtmlStringSink() { - exists(ObjectCreation c, MicrosoftAspNetCoreHttpHtmlString s | - c.getTarget() = s.getAConstructor() and - this.asExpr() = c.getAnArgument() - ) - } - } } private Type getMemberType(Member m) { diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/Email.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Email.qll similarity index 88% rename from csharp/ql/src/semmle/code/csharp/security/dataflow/Email.qll rename to csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Email.qll index b86fa4822ae..c45bd9e8b39 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/Email.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Email.qll @@ -1,11 +1,13 @@ /** Provides data flow sinks for sending email. */ import csharp +private import Remote private import semmle.code.csharp.frameworks.system.net.Mail +/** Provides sinks for emails. */ module Email { /** A data flow sink for sending email. */ - abstract class Sink extends DataFlow::ExprNode { } + abstract class Sink extends DataFlow::ExprNode, RemoteFlowSink { } /** A data flow sink for sending email via `System.Net.Mail.MailMessage`. */ class MailMessageSink extends Sink { diff --git a/csharp/ql/src/semmle/code/csharp/security/sinks/ExternalLocationSink.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll similarity index 95% rename from csharp/ql/src/semmle/code/csharp/security/sinks/ExternalLocationSink.qll rename to csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll index 6cd1a5cb269..25a50f3733c 100644 --- a/csharp/ql/src/semmle/code/csharp/security/sinks/ExternalLocationSink.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll @@ -3,6 +3,7 @@ */ import csharp +private import Remote private import semmle.code.csharp.commons.Loggers private import semmle.code.csharp.frameworks.system.Web @@ -45,7 +46,7 @@ class TraceMessageSink extends ExternalLocationSink { /** * An expression set as a value on a cookie instance. */ -class CookieStorageSink extends ExternalLocationSink { +class CookieStorageSink extends ExternalLocationSink, RemoteFlowSink { CookieStorageSink() { exists(Expr e | e = this.getExpr() | e = any(SystemWebHttpCookie cookie).getAConstructor().getACall().getArgumentForName("value") diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Html.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Html.qll new file mode 100644 index 00000000000..2363e24ffeb --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Html.qll @@ -0,0 +1,208 @@ +/** + * Provides classes representing HTML data flow sinks. + */ + +import csharp +private import Remote +private import semmle.code.csharp.frameworks.microsoft.AspNetCore +private import semmle.code.csharp.frameworks.system.Net +private import semmle.code.csharp.frameworks.system.Web +private import semmle.code.csharp.frameworks.system.web.Mvc +private import semmle.code.csharp.frameworks.system.web.WebPages +private import semmle.code.csharp.frameworks.system.web.UI +private import semmle.code.csharp.frameworks.system.web.ui.WebControls +private import semmle.code.csharp.frameworks.system.windows.Forms +private import semmle.code.csharp.security.dataflow.flowsources.Remote +private import semmle.code.asp.AspNet + +/** + * A sink where the value of the expression may be rendered as HTML, + * without implicit HTML encoding. + */ +abstract class HtmlSink extends DataFlow::ExprNode, RemoteFlowSink { } + +/** + * An expression that is used as an argument to an HTML sink method on + * `HttpResponse`. + */ +class HttpResponseSink extends HtmlSink { + HttpResponseSink() { + exists(Method m, SystemWebHttpResponseClass responseClass | + m = responseClass.getAWriteMethod() or + m = responseClass.getAWriteFileMethod() or + m = responseClass.getATransmitFileMethod() or + m = responseClass.getABinaryWriteMethod() + | + // Calls to these methods, or overrides of them + this.getExpr() = m.getAnOverrider*().getParameter(0).getAnAssignedArgument() + ) + } +} + +/** + * An expression that is used as an argument to an HTML sink method on + * `HtmlTextWriter`. + */ +class HtmlTextWriterSink extends HtmlSink { + HtmlTextWriterSink() { + exists(SystemWebUIHtmlTextWriterClass writeClass, Method m, Call c, int paramPos | + paramPos = 0 and + ( + m = writeClass.getAWriteMethod() or + m = writeClass.getAWriteLineMethod() or + m = writeClass.getAWriteLineNoTabsMethod() or + m = writeClass.getAWriteBeginTagMethod() or + m = writeClass.getAWriteAttributeMethod() + ) + or + // The second parameter to the `WriteAttribute` method is the attribute value, which we + // should only consider as tainted if the call does not ask for the attribute value to be + // encoded using the final parameter. + m = writeClass.getAWriteAttributeMethod() and + paramPos = 1 and + not c.getArgumentForParameter(m.getParameter(2)).(BoolLiteral).getBoolValue() = true + | + c = m.getACall() and + this.getExpr() = c.getArgumentForParameter(m.getParameter(paramPos)) + ) + } +} + +/** + * An expression that is used as an argument to an HTML sink method on + * `AttributeCollection`. + */ +class AttributeCollectionSink extends HtmlSink { + AttributeCollectionSink() { + exists(SystemWebUIAttributeCollectionClass ac, Parameter p | + p = ac.getAddMethod().getParameter(1) or + p = ac.getItemProperty().getSetter().getParameter(0) + | + this.getExpr() = p.getAnAssignedArgument() + ) + } +} + +/** + * An expression that is used as the second argument `HtmlElement.SetAttribute`. + */ +class SetAttributeSink extends HtmlSink { + SetAttributeSink() { + this.getExpr() = + any(SystemWindowsFormsHtmlElement c).getSetAttributeMethod().getACall().getArgument(1) + } +} + +/** + * An expression that is used as an argument to an HTML sink setter, on + * a class within the `System.Web.UI` namespace. + */ +class SystemWebSetterHtmlSink extends HtmlSink { + SystemWebSetterHtmlSink() { + exists(Property p, string name, ValueOrRefType declaringType | + declaringType = p.getDeclaringType() and + any(SystemWebUINamespace n).getAChildNamespace*() = declaringType.getNamespace() and + this.getExpr() = p.getAnAssignedValue() and + p.hasName(name) + | + name = "Caption" and + (declaringType.hasName("Calendar") or declaringType.hasName("Table")) + or + name = "InnerHtml" + ) + or + exists(SystemWebUIWebControlsLabelClass c | + // Unlike `Text` properties of other web controls, `Label.Text` is not automatically HTML encoded + this.getExpr() = c.getTextProperty().getSetter().getParameter(0).getAnAssignedArgument() + ) + } +} + +/** + * An expression that is used as an argument to `HtmlHelper.Raw`, typically in + * a `.cshtml` file. + */ +class SystemWebMvcHtmlHelperRawSink extends HtmlSink { + SystemWebMvcHtmlHelperRawSink() { + this.getExpr() = any(SystemWebMvcHtmlHelperClass h).getRawMethod().getACall().getAnArgument() + } +} + +/** An expression that is returned from a `ToHtmlString` method. */ +class ToHtmlString extends HtmlSink { + ToHtmlString() { + exists(Method toHtmlString | + toHtmlString = any(SystemWebIHtmlString i).getToHtmlStringMethod().getAnUltimateImplementor() and + toHtmlString.canReturn(this.getExpr()) + ) + } +} + +/** + * An expression passed to the constructor of an `HtmlString` or a `MvcHtmlString`. + */ +class HtmlString extends HtmlSink { + HtmlString() { + exists(Class c | + c = any(SystemWebMvcMvcHtmlString m) or + c = any(SystemWebHtmlString m) + | + this.getExpr() = c.getAConstructor().getACall().getAnArgument() + ) + } +} + +/** + * An expression that is used as an argument to `Page.WriteLiteral`, typically in + * a `.cshtml` file. + */ +class WebPageWriteLiteralSink extends HtmlSink { + WebPageWriteLiteralSink() { + this.getExpr() = any(WebPageClass h).getWriteLiteralMethod().getACall().getAnArgument() + } +} + +/** + * An expression that is used as an argument to `Page.WriteLiteralTo`, typically in + * a `.cshtml` file. + */ +class WebPageWriteLiteralToSink extends HtmlSink { + WebPageWriteLiteralToSink() { + this.getExpr() = any(WebPageClass h).getWriteLiteralToMethod().getACall().getAnArgument() + } +} + +/** An ASP.NET Core HTML sink. */ +abstract class AspNetCoreHtmlSink extends HtmlSink { } + +/** + * An expression that is used as an argument to `HtmlHelper.Raw`, typically in + * a `.cshtml` file. + */ +class MicrosoftAspNetCoreMvcHtmlHelperRawSink extends AspNetCoreHtmlSink { + MicrosoftAspNetCoreMvcHtmlHelperRawSink() { + this.getExpr() = + any(MicrosoftAspNetCoreMvcHtmlHelperClass h).getRawMethod().getACall().getAnArgument() + } +} + +/** + * An expression that is used as an argument to `Page.WriteLiteral` in ASP.NET 6.0 razor page, typically in + * a `.cshtml` file. + */ +class MicrosoftAspNetRazorPageWriteLiteralSink extends AspNetCoreHtmlSink { + MicrosoftAspNetRazorPageWriteLiteralSink() { + this.getExpr() = + any(MicrosoftAspNetCoreMvcRazorPageBase h).getWriteLiteralMethod().getACall().getAnArgument() + } +} + +/** `HtmlString` that may be rendered as is need to have sanitized value. */ +class MicrosoftAspNetHtmlStringSink extends AspNetCoreHtmlSink { + MicrosoftAspNetHtmlStringSink() { + exists(ObjectCreation c, MicrosoftAspNetCoreHttpHtmlString s | + c.getTarget() = s.getAConstructor() and + this.asExpr() = c.getAnArgument() + ) + } +} diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Remote.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Remote.qll new file mode 100644 index 00000000000..10885d52a16 --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Remote.qll @@ -0,0 +1,31 @@ +/** + * Provides classes representing data flow sinks for remote user output. + */ + +import csharp +private import Email::Email +private import ExternalLocationSink +private import Html +private import semmle.code.csharp.security.dataflow.XSS +private import semmle.code.csharp.frameworks.system.web.UI + +/** A data flow sink of remote user output. */ +abstract class RemoteFlowSink extends DataFlow::Node { } + +/** + * A value written to the `[Inner]Text` property of an object defined in the + * `System.Web.UI` namespace. + */ +class SystemWebUIText extends RemoteFlowSink { + SystemWebUIText() { + exists(Property p, string name | + p.getDeclaringType().getNamespace().getParentNamespace*() instanceof SystemWebUINamespace and + this.asExpr() = p.getAnAssignedValue() and + p.hasName(name) + | + name = "Text" + or + name = "InnerText" + ) + } +} diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Local.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll similarity index 100% rename from csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Local.qll rename to csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Remote.qll new file mode 100644 index 00000000000..f8240583108 --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Remote.qll @@ -0,0 +1,218 @@ +/** + * Provides classes representing data flow sources for remote user input. + */ + +import csharp +private import semmle.code.csharp.frameworks.system.Net +private import semmle.code.csharp.frameworks.system.Web +private import semmle.code.csharp.frameworks.system.web.Http +private import semmle.code.csharp.frameworks.system.web.Mvc +private import semmle.code.csharp.frameworks.system.web.Services +private import semmle.code.csharp.frameworks.system.web.ui.WebControls +private import semmle.code.csharp.frameworks.WCF +private import semmle.code.csharp.frameworks.microsoft.Owin +private import semmle.code.csharp.frameworks.microsoft.AspNetCore + +/** A data flow source of remote user input. */ +abstract class RemoteFlowSource extends DataFlow::Node { + /** Gets a string that describes the type of this remote flow source. */ + abstract string getSourceType(); +} + +/** A data flow source of remote user input (ASP.NET). */ +abstract class AspNetRemoteFlowSource extends RemoteFlowSource { } + +/** A member containing an ASP.NET query string. */ +class AspNetQueryStringMember extends Member { + AspNetQueryStringMember() { + exists(RefType t | + t instanceof SystemWebHttpRequestClass or + t instanceof SystemNetHttpListenerRequestClass or + t instanceof SystemWebHttpRequestBaseClass + | + this = t.getProperty(getHttpRequestFlowPropertyNames()) or + this.(Field).getType() = t or + this.(Property).getType() = t or + this.(Callable).getReturnType() = t + ) + } +} + +/** + * Gets the names of the properties in `HttpRequest` classes that should propagate taint out of the + * request. + */ +private string getHttpRequestFlowPropertyNames() { + result = "QueryString" or + result = "Headers" or + result = "RawUrl" or + result = "Url" or + result = "Cookies" or + result = "Form" or + result = "Params" or + result = "Path" or + result = "PathInfo" +} + +/** A data flow source of remote user input (ASP.NET query string). */ +class AspNetQueryStringRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode { + AspNetQueryStringRemoteFlowSource() { + exists(RefType t | + t instanceof SystemWebHttpRequestClass or + t instanceof SystemNetHttpListenerRequestClass or + t instanceof SystemWebHttpRequestBaseClass + | + // A request object can be indexed, so taint the object as well + this.getExpr().getType() = t + ) + or + this.getExpr() = any(AspNetQueryStringMember m).getAnAccess() + } + + override string getSourceType() { result = "ASP.NET query string" } +} + +/** A data flow source of remote user input (ASP.NET unvalidated request data). */ +class AspNetUnvalidatedQueryStringRemoteFlowSource extends AspNetRemoteFlowSource, + DataFlow::ExprNode { + AspNetUnvalidatedQueryStringRemoteFlowSource() { + this.getExpr() = any(SystemWebUnvalidatedRequestValues c).getAProperty().getGetter().getACall() or + this.getExpr() = + any(SystemWebUnvalidatedRequestValuesBase c).getAProperty().getGetter().getACall() + } + + override string getSourceType() { result = "ASP.NET unvalidated request data" } +} + +/** A data flow source of remote user input (ASP.NET user input). */ +class AspNetUserInputRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode { + AspNetUserInputRemoteFlowSource() { getType() instanceof SystemWebUIWebControlsTextBoxClass } + + override string getSourceType() { result = "ASP.NET user input" } +} + +/** A data flow source of remote user input (WCF based web service). */ +class WcfRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { + WcfRemoteFlowSource() { exists(OperationMethod om | om.getAParameter() = this.getParameter()) } + + override string getSourceType() { result = "web service input" } +} + +/** A data flow source of remote user input (ASP.NET web service). */ +class AspNetServiceRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { + AspNetServiceRemoteFlowSource() { + exists(Method m | + m.getAParameter() = this.getParameter() and + m.getAnAttribute().getType() instanceof SystemWebServicesWebMethodAttributeClass + ) + } + + override string getSourceType() { result = "ASP.NET web service input" } +} + +/** A data flow source of remote user input (ASP.NET request message). */ +class SystemNetHttpRequestMessageRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode { + SystemNetHttpRequestMessageRemoteFlowSource() { + getType() instanceof SystemWebHttpRequestMessageClass + } + + override string getSourceType() { result = "ASP.NET request message" } +} + +/** + * A data flow source of remote user input (Microsoft Owin, a query, request, + * or path string). + */ +class MicrosoftOwinStringFlowSource extends RemoteFlowSource, DataFlow::ExprNode { + MicrosoftOwinStringFlowSource() { + this.getExpr() = any(MicrosoftOwinString owinString).getValueProperty().getGetter().getACall() + } + + override string getSourceType() { result = "Microsoft Owin request or query string" } +} + +/** A data flow source of remote user input (`Microsoft Owin IOwinRequest`). */ +class MicrosoftOwinRequestRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode { + MicrosoftOwinRequestRemoteFlowSource() { + exists(Property p, MicrosoftOwinIOwinRequestClass owinRequest | + this.getExpr() = p.getGetter().getACall() + | + p = owinRequest.getAcceptProperty() or + p = owinRequest.getBodyProperty() or + p = owinRequest.getCacheControlProperty() or + p = owinRequest.getContentTypeProperty() or + p = owinRequest.getContextProperty() or + p = owinRequest.getCookiesProperty() or + p = owinRequest.getHeadersProperty() or + p = owinRequest.getHostProperty() or + p = owinRequest.getMediaTypeProperty() or + p = owinRequest.getMethodProperty() or + p = owinRequest.getPathProperty() or + p = owinRequest.getPathBaseProperty() or + p = owinRequest.getQueryProperty() or + p = owinRequest.getQueryStringProperty() or + p = owinRequest.getRemoteIpAddressProperty() or + p = owinRequest.getSchemeProperty() or + p = owinRequest.getURIProperty() + ) + } + + override string getSourceType() { result = "Microsoft Owin request" } +} + +/** A parameter to an Mvc controller action method, viewed as a source of remote user input. */ +class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { + ActionMethodParameter() { + exists(Parameter p | + p = this.getParameter() and + p.fromSource() + | + p = any(Controller c).getAnActionMethod().getAParameter() or + p = any(ApiController c).getAnActionMethod().getAParameter() + ) + } + + override string getSourceType() { result = "ASP.NET MVC action method parameter" } +} + +/** A data flow source of remote user input (ASP.NET Core). */ +abstract class AspNetCoreRemoteFlowSource extends RemoteFlowSource { } + +/** A data flow source of remote user input (ASP.NET query collection). */ +class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFlow::ExprNode { + AspNetCoreQueryRemoteFlowSource() { + exists(ValueOrRefType t | + t instanceof MicrosoftAspNetCoreHttpHttpRequest or + t instanceof MicrosoftAspNetCoreHttpQueryCollection or + t instanceof MicrosoftAspNetCoreHttpQueryString + | + this.getExpr().(Call).getTarget().getDeclaringType() = t or + this.asExpr().(Access).getTarget().getDeclaringType() = t + ) + or + exists(Call c | + c + .getTarget() + .getDeclaringType() + .hasQualifiedName("Microsoft.AspNetCore.Http", "IQueryCollection") and + c.getTarget().getName() = "TryGetValue" and + this.asExpr() = c.getArgumentForName("value") + ) + } + + override string getSourceType() { result = "ASP.NET Core query string" } +} + +/** A parameter to a `Mvc` controller action method, viewed as a source of remote user input. */ +class AspNetCoreActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { + AspNetCoreActionMethodParameter() { + exists(Parameter p | + p = this.getParameter() and + p.fromSource() + | + p = any(MicrosoftAspNetCoreMvcController c).getAnActionMethod().getAParameter() + ) + } + + override string getSourceType() { result = "ASP.NET Core MVC action method parameter" } +} diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql index 4015799bd1a..29281ffba56 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql @@ -1,4 +1,4 @@ -import semmle.code.csharp.dataflow.flowsources.Remote +import semmle.code.csharp.security.dataflow.flowsources.Remote from RemoteFlowSource source select source, source.getSourceType() diff --git a/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.cs b/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.cs index 2a7cc3ae232..c0d56d1337e 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.cs +++ b/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.cs @@ -2,10 +2,13 @@ using System; using System.Web; +using System.Web.UI.WebControls; public class StackTraceHandler : IHttpHandler { bool b; + TextBox textBox; + public void ProcessRequest(HttpContext ctx) { try @@ -34,6 +37,11 @@ public class StackTraceHandler : IHttpHandler // GOOD: log the stack trace, and send back a non-revealing response log("Exception occurred", ex); ctx.Response.Write("Exception occurred"); + + textBox.Text = ex.InnerException.StackTrace; // BAD + textBox.Text = ex.StackTrace; // BAD + textBox.Text = ex.ToString(); // BAD + textBox.Text = ex.Message; // GOOD return; } diff --git a/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.expected b/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.expected index 3cdd0f50449..492a61dd038 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.expected +++ b/csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.expected @@ -1,14 +1,20 @@ edges -| ExceptionInformationExposure.cs:18:32:18:33 | access to local variable ex : Exception | ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | +| ExceptionInformationExposure.cs:21:32:21:33 | access to local variable ex : Exception | ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | nodes -| ExceptionInformationExposure.cs:18:32:18:33 | access to local variable ex : Exception | semmle.label | access to local variable ex : Exception | -| ExceptionInformationExposure.cs:18:32:18:44 | call to method ToString | semmle.label | call to method ToString | -| ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | semmle.label | access to local variable ex | -| ExceptionInformationExposure.cs:22:32:22:44 | access to property StackTrace | semmle.label | access to property StackTrace | -| ExceptionInformationExposure.cs:41:28:41:55 | call to method ToString | semmle.label | call to method ToString | +| ExceptionInformationExposure.cs:21:32:21:33 | access to local variable ex : Exception | semmle.label | access to local variable ex : Exception | +| ExceptionInformationExposure.cs:21:32:21:44 | call to method ToString | semmle.label | call to method ToString | +| ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | semmle.label | access to local variable ex | +| ExceptionInformationExposure.cs:25:32:25:44 | access to property StackTrace | semmle.label | access to property StackTrace | +| ExceptionInformationExposure.cs:41:28:41:55 | access to property StackTrace | semmle.label | access to property StackTrace | +| ExceptionInformationExposure.cs:42:28:42:40 | access to property StackTrace | semmle.label | access to property StackTrace | +| ExceptionInformationExposure.cs:43:28:43:40 | call to method ToString | semmle.label | call to method ToString | +| ExceptionInformationExposure.cs:49:28:49:55 | call to method ToString | semmle.label | call to method ToString | #select -| ExceptionInformationExposure.cs:18:32:18:44 | call to method ToString | ExceptionInformationExposure.cs:18:32:18:44 | call to method ToString | ExceptionInformationExposure.cs:18:32:18:44 | call to method ToString | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:18:32:18:44 | call to method ToString | call to method ToString | -| ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | ExceptionInformationExposure.cs:18:32:18:33 | access to local variable ex : Exception | ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:18:32:18:33 | access to local variable ex | access to local variable ex : Exception | -| ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:20:32:20:33 | access to local variable ex | access to local variable ex | -| ExceptionInformationExposure.cs:22:32:22:44 | access to property StackTrace | ExceptionInformationExposure.cs:22:32:22:44 | access to property StackTrace | ExceptionInformationExposure.cs:22:32:22:44 | access to property StackTrace | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:22:32:22:44 | access to property StackTrace | access to property StackTrace | -| ExceptionInformationExposure.cs:41:28:41:55 | call to method ToString | ExceptionInformationExposure.cs:41:28:41:55 | call to method ToString | ExceptionInformationExposure.cs:41:28:41:55 | call to method ToString | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:41:28:41:55 | call to method ToString | call to method ToString | +| ExceptionInformationExposure.cs:21:32:21:44 | call to method ToString | ExceptionInformationExposure.cs:21:32:21:44 | call to method ToString | ExceptionInformationExposure.cs:21:32:21:44 | call to method ToString | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:21:32:21:44 | call to method ToString | call to method ToString | +| ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | ExceptionInformationExposure.cs:21:32:21:33 | access to local variable ex : Exception | ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:21:32:21:33 | access to local variable ex | access to local variable ex : Exception | +| ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | access to local variable ex | +| ExceptionInformationExposure.cs:25:32:25:44 | access to property StackTrace | ExceptionInformationExposure.cs:25:32:25:44 | access to property StackTrace | ExceptionInformationExposure.cs:25:32:25:44 | access to property StackTrace | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:25:32:25:44 | access to property StackTrace | access to property StackTrace | +| ExceptionInformationExposure.cs:41:28:41:55 | access to property StackTrace | ExceptionInformationExposure.cs:41:28:41:55 | access to property StackTrace | ExceptionInformationExposure.cs:41:28:41:55 | access to property StackTrace | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:41:28:41:55 | access to property StackTrace | access to property StackTrace | +| ExceptionInformationExposure.cs:42:28:42:40 | access to property StackTrace | ExceptionInformationExposure.cs:42:28:42:40 | access to property StackTrace | ExceptionInformationExposure.cs:42:28:42:40 | access to property StackTrace | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:42:28:42:40 | access to property StackTrace | access to property StackTrace | +| ExceptionInformationExposure.cs:43:28:43:40 | call to method ToString | ExceptionInformationExposure.cs:43:28:43:40 | call to method ToString | ExceptionInformationExposure.cs:43:28:43:40 | call to method ToString | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:43:28:43:40 | call to method ToString | call to method ToString | +| ExceptionInformationExposure.cs:49:28:49:55 | call to method ToString | ExceptionInformationExposure.cs:49:28:49:55 | call to method ToString | ExceptionInformationExposure.cs:49:28:49:55 | call to method ToString | Exception information from $@ flows to here, and is exposed to the user. | ExceptionInformationExposure.cs:49:28:49:55 | call to method ToString | call to method ToString |