Skip to content

Commit

Permalink
Merge pull request #3124 from hvitved/csharp/dataflow/sources-and-sinks
Browse files Browse the repository at this point in the history
C#: Introduce `RemoteFlowSink` class
  • Loading branch information
calumgrant authored Apr 6, 2020
2 parents 5034d40 + c8c706a commit 6cce0de
Show file tree
Hide file tree
Showing 37 changed files with 553 additions and 477 deletions.
3 changes: 3 additions & 0 deletions change-notes/1.24/analysis-csharp.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
2 changes: 1 addition & 1 deletion csharp/ql/src/Security Features/CWE-091/XMLInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Security Features/InsecureRandomness.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
219 changes: 4 additions & 215 deletions csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 6cce0de

Please sign in to comment.