Skip to content

Commit 6cce0de

Browse files
authored
Merge pull request github#3124 from hvitved/csharp/dataflow/sources-and-sinks
C#: Introduce `RemoteFlowSink` class
2 parents 5034d40 + c8c706a commit 6cce0de

37 files changed

+553
-477
lines changed

change-notes/1.24/analysis-csharp.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ The following changes in version 1.24 affect C# analysis in all applications.
2222
| Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | More results | Results are reported from parameters with a default value of `null`. |
2323
| 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`. |
2424
| XPath injection (`cs/xml/xpath-injection`) | More results | The query now recognizes calls to methods on `System.Xml.XPath.XPathNavigator` objects. |
25+
| 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. |
26+
| 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. |
2527

2628
## Removal of old queries
2729

@@ -42,5 +44,6 @@ The following changes in version 1.24 affect C# analysis in all applications.
4244
* [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.
4345
* Expression nullability flow state is given by the predicates `Expr.hasNotNullFlowState()` and `Expr.hasMaybeNullFlowState()`.
4446
* `stackalloc` array creations are now represented by the QL class `Stackalloc`. Previously they were represented by the class `ArrayCreation`.
47+
* 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.
4548

4649
## Changes to autobuilder

csharp/ql/src/Security Features/CWE-091/XMLInjection.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*/
1212

1313
import csharp
14-
import semmle.code.csharp.dataflow.flowsources.Remote
14+
import semmle.code.csharp.security.dataflow.flowsources.Remote
1515
import semmle.code.csharp.frameworks.system.Xml
1616

1717
/**

csharp/ql/src/Security Features/CWE-114/AssemblyPathInjection.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
*/
1313

1414
import csharp
15-
import semmle.code.csharp.dataflow.flowsources.Remote
15+
import semmle.code.csharp.security.dataflow.flowsources.Remote
1616
import semmle.code.csharp.commons.Util
1717

1818
/**

csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
*/
1212

1313
import csharp
14-
import semmle.code.csharp.dataflow.flowsources.Remote
15-
import semmle.code.csharp.dataflow.flowsources.Local
14+
import semmle.code.csharp.security.dataflow.flowsources.Remote
15+
import semmle.code.csharp.security.dataflow.flowsources.Local
1616
import semmle.code.csharp.dataflow.TaintTracking
1717
import semmle.code.csharp.frameworks.Format
1818
import DataFlow::PathGraph

csharp/ql/src/Security Features/CWE-201/ExposureInTransmittedData.ql

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@
1111

1212
import csharp
1313
import semmle.code.csharp.security.SensitiveActions
14-
import semmle.code.csharp.security.dataflow.XSS
15-
import semmle.code.csharp.security.dataflow.Email
14+
import semmle.code.csharp.security.dataflow.flowsinks.Remote
1615
import semmle.code.csharp.frameworks.system.data.Common
1716
import semmle.code.csharp.frameworks.System
1817
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
@@ -42,11 +41,7 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
4241
)
4342
}
4443

45-
override predicate isSink(DataFlow::Node sink) {
46-
sink instanceof XSS::Sink
47-
or
48-
sink instanceof Email::Sink
49-
}
44+
override predicate isSink(DataFlow::Node sink) { sink instanceof RemoteFlowSink }
5045
}
5146

5247
from TaintTrackingConfiguration configuration, DataFlow::PathNode source, DataFlow::PathNode sink

csharp/ql/src/Security Features/CWE-209/ExceptionInformationExposure.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import csharp
1616
import semmle.code.csharp.frameworks.System
17-
import semmle.code.csharp.security.dataflow.XSS
17+
import semmle.code.csharp.security.dataflow.flowsinks.Remote
1818
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
1919

2020
/**
@@ -46,7 +46,7 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
4646
)
4747
}
4848

49-
override predicate isSink(DataFlow::Node sink) { sink instanceof XSS::Sink }
49+
override predicate isSink(DataFlow::Node sink) { sink instanceof RemoteFlowSink }
5050

5151
override predicate isSanitizer(DataFlow::Node sanitizer) {
5252
// Do not flow through Message

csharp/ql/src/Security Features/CWE-838/InappropriateEncoding.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import semmle.code.csharp.frameworks.system.Net
1616
import semmle.code.csharp.frameworks.system.Web
1717
import semmle.code.csharp.frameworks.system.web.UI
1818
import semmle.code.csharp.security.dataflow.SqlInjection
19-
import semmle.code.csharp.security.dataflow.XSS
19+
import semmle.code.csharp.security.dataflow.flowsinks.Html
2020
import semmle.code.csharp.security.dataflow.UrlRedirect
2121
import semmle.code.csharp.security.Sanitizers
2222
import semmle.code.csharp.dataflow.DataFlow2::DataFlow2
@@ -114,7 +114,7 @@ module EncodingConfigurations {
114114

115115
override string getKind() { result = "HTML expression" }
116116

117-
override predicate requiresEncoding(Node n) { n instanceof XSS::HtmlSink }
117+
override predicate requiresEncoding(Node n) { n instanceof HtmlSink }
118118

119119
override predicate isPossibleEncodedValue(Expr e) { e instanceof HtmlSanitizedExpr }
120120
}

csharp/ql/src/Security Features/InsecureRandomness.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import semmle.code.csharp.frameworks.Test
1616
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
1717

1818
module Random {
19-
import semmle.code.csharp.dataflow.flowsources.Remote
19+
import semmle.code.csharp.security.dataflow.flowsources.Remote
2020
import semmle.code.csharp.security.SensitiveActions
2121

2222
/**

csharp/ql/src/semmle/code/csharp/dataflow/flowsources/PublicCallableParameter.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
/**
2+
* DEPRECATED.
3+
*
24
* Provides classes representing data flow sources for parameters of public callables.
35
*/
46

57
import csharp
6-
private import semmle.code.csharp.frameworks.WCF
78

89
/**
910
* A parameter of a public callable, for example `p` in
Lines changed: 4 additions & 215 deletions
Original file line numberDiff line numberDiff line change
@@ -1,218 +1,7 @@
11
/**
2-
* Provides classes representing data flow sources for remote user input.
2+
* DEPRECATED.
3+
*
4+
* Use `semmle.code.csharp.security.dataflow.flowsources.Remote` instead.
35
*/
46

5-
import csharp
6-
private import semmle.code.csharp.frameworks.system.Net
7-
private import semmle.code.csharp.frameworks.system.Web
8-
private import semmle.code.csharp.frameworks.system.web.Http
9-
private import semmle.code.csharp.frameworks.system.web.Mvc
10-
private import semmle.code.csharp.frameworks.system.web.Services
11-
private import semmle.code.csharp.frameworks.system.web.ui.WebControls
12-
private import semmle.code.csharp.frameworks.WCF
13-
private import semmle.code.csharp.frameworks.microsoft.Owin
14-
private import semmle.code.csharp.frameworks.microsoft.AspNetCore
15-
16-
/** A data flow source of remote user input. */
17-
abstract class RemoteFlowSource extends DataFlow::Node {
18-
/** Gets a string that describes the type of this remote flow source. */
19-
abstract string getSourceType();
20-
}
21-
22-
/** A data flow source of remote user input (ASP.NET). */
23-
abstract class AspNetRemoteFlowSource extends RemoteFlowSource { }
24-
25-
/** A member containing an ASP.NET query string. */
26-
class AspNetQueryStringMember extends Member {
27-
AspNetQueryStringMember() {
28-
exists(RefType t |
29-
t instanceof SystemWebHttpRequestClass or
30-
t instanceof SystemNetHttpListenerRequestClass or
31-
t instanceof SystemWebHttpRequestBaseClass
32-
|
33-
this = t.getProperty(getHttpRequestFlowPropertyNames()) or
34-
this.(Field).getType() = t or
35-
this.(Property).getType() = t or
36-
this.(Callable).getReturnType() = t
37-
)
38-
}
39-
}
40-
41-
/**
42-
* Gets the names of the properties in `HttpRequest` classes that should propagate taint out of the
43-
* request.
44-
*/
45-
private string getHttpRequestFlowPropertyNames() {
46-
result = "QueryString" or
47-
result = "Headers" or
48-
result = "RawUrl" or
49-
result = "Url" or
50-
result = "Cookies" or
51-
result = "Form" or
52-
result = "Params" or
53-
result = "Path" or
54-
result = "PathInfo"
55-
}
56-
57-
/** A data flow source of remote user input (ASP.NET query string). */
58-
class AspNetQueryStringRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode {
59-
AspNetQueryStringRemoteFlowSource() {
60-
exists(RefType t |
61-
t instanceof SystemWebHttpRequestClass or
62-
t instanceof SystemNetHttpListenerRequestClass or
63-
t instanceof SystemWebHttpRequestBaseClass
64-
|
65-
// A request object can be indexed, so taint the object as well
66-
this.getExpr().getType() = t
67-
)
68-
or
69-
this.getExpr() = any(AspNetQueryStringMember m).getAnAccess()
70-
}
71-
72-
override string getSourceType() { result = "ASP.NET query string" }
73-
}
74-
75-
/** A data flow source of remote user input (ASP.NET unvalidated request data). */
76-
class AspNetUnvalidatedQueryStringRemoteFlowSource extends AspNetRemoteFlowSource,
77-
DataFlow::ExprNode {
78-
AspNetUnvalidatedQueryStringRemoteFlowSource() {
79-
this.getExpr() = any(SystemWebUnvalidatedRequestValues c).getAProperty().getGetter().getACall() or
80-
this.getExpr() =
81-
any(SystemWebUnvalidatedRequestValuesBase c).getAProperty().getGetter().getACall()
82-
}
83-
84-
override string getSourceType() { result = "ASP.NET unvalidated request data" }
85-
}
86-
87-
/** A data flow source of remote user input (ASP.NET user input). */
88-
class AspNetUserInputRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode {
89-
AspNetUserInputRemoteFlowSource() { getType() instanceof SystemWebUIWebControlsTextBoxClass }
90-
91-
override string getSourceType() { result = "ASP.NET user input" }
92-
}
93-
94-
/** A data flow source of remote user input (WCF based web service). */
95-
class WcfRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode {
96-
WcfRemoteFlowSource() { exists(OperationMethod om | om.getAParameter() = this.getParameter()) }
97-
98-
override string getSourceType() { result = "web service input" }
99-
}
100-
101-
/** A data flow source of remote user input (ASP.NET web service). */
102-
class AspNetServiceRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode {
103-
AspNetServiceRemoteFlowSource() {
104-
exists(Method m |
105-
m.getAParameter() = this.getParameter() and
106-
m.getAnAttribute().getType() instanceof SystemWebServicesWebMethodAttributeClass
107-
)
108-
}
109-
110-
override string getSourceType() { result = "ASP.NET web service input" }
111-
}
112-
113-
/** A data flow source of remote user input (ASP.NET request message). */
114-
class SystemNetHttpRequestMessageRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode {
115-
SystemNetHttpRequestMessageRemoteFlowSource() {
116-
getType() instanceof SystemWebHttpRequestMessageClass
117-
}
118-
119-
override string getSourceType() { result = "ASP.NET request message" }
120-
}
121-
122-
/**
123-
* A data flow source of remote user input (Microsoft Owin, a query, request,
124-
* or path string).
125-
*/
126-
class MicrosoftOwinStringFlowSource extends RemoteFlowSource, DataFlow::ExprNode {
127-
MicrosoftOwinStringFlowSource() {
128-
this.getExpr() = any(MicrosoftOwinString owinString).getValueProperty().getGetter().getACall()
129-
}
130-
131-
override string getSourceType() { result = "Microsoft Owin request or query string" }
132-
}
133-
134-
/** A data flow source of remote user input (`Microsoft Owin IOwinRequest`). */
135-
class MicrosoftOwinRequestRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode {
136-
MicrosoftOwinRequestRemoteFlowSource() {
137-
exists(Property p, MicrosoftOwinIOwinRequestClass owinRequest |
138-
this.getExpr() = p.getGetter().getACall()
139-
|
140-
p = owinRequest.getAcceptProperty() or
141-
p = owinRequest.getBodyProperty() or
142-
p = owinRequest.getCacheControlProperty() or
143-
p = owinRequest.getContentTypeProperty() or
144-
p = owinRequest.getContextProperty() or
145-
p = owinRequest.getCookiesProperty() or
146-
p = owinRequest.getHeadersProperty() or
147-
p = owinRequest.getHostProperty() or
148-
p = owinRequest.getMediaTypeProperty() or
149-
p = owinRequest.getMethodProperty() or
150-
p = owinRequest.getPathProperty() or
151-
p = owinRequest.getPathBaseProperty() or
152-
p = owinRequest.getQueryProperty() or
153-
p = owinRequest.getQueryStringProperty() or
154-
p = owinRequest.getRemoteIpAddressProperty() or
155-
p = owinRequest.getSchemeProperty() or
156-
p = owinRequest.getURIProperty()
157-
)
158-
}
159-
160-
override string getSourceType() { result = "Microsoft Owin request" }
161-
}
162-
163-
/** A parameter to an Mvc controller action method, viewed as a source of remote user input. */
164-
class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode {
165-
ActionMethodParameter() {
166-
exists(Parameter p |
167-
p = this.getParameter() and
168-
p.fromSource()
169-
|
170-
p = any(Controller c).getAnActionMethod().getAParameter() or
171-
p = any(ApiController c).getAnActionMethod().getAParameter()
172-
)
173-
}
174-
175-
override string getSourceType() { result = "ASP.NET MVC action method parameter" }
176-
}
177-
178-
/** A data flow source of remote user input (ASP.NET Core). */
179-
abstract class AspNetCoreRemoteFlowSource extends RemoteFlowSource { }
180-
181-
/** A data flow source of remote user input (ASP.NET query collection). */
182-
class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFlow::ExprNode {
183-
AspNetCoreQueryRemoteFlowSource() {
184-
exists(ValueOrRefType t |
185-
t instanceof MicrosoftAspNetCoreHttpHttpRequest or
186-
t instanceof MicrosoftAspNetCoreHttpQueryCollection or
187-
t instanceof MicrosoftAspNetCoreHttpQueryString
188-
|
189-
this.getExpr().(Call).getTarget().getDeclaringType() = t or
190-
this.asExpr().(Access).getTarget().getDeclaringType() = t
191-
)
192-
or
193-
exists(Call c |
194-
c
195-
.getTarget()
196-
.getDeclaringType()
197-
.hasQualifiedName("Microsoft.AspNetCore.Http", "IQueryCollection") and
198-
c.getTarget().getName() = "TryGetValue" and
199-
this.asExpr() = c.getArgumentForName("value")
200-
)
201-
}
202-
203-
override string getSourceType() { result = "ASP.NET Core query string" }
204-
}
205-
206-
/** A parameter to a `Mvc` controller action method, viewed as a source of remote user input. */
207-
class AspNetCoreActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode {
208-
AspNetCoreActionMethodParameter() {
209-
exists(Parameter p |
210-
p = this.getParameter() and
211-
p.fromSource()
212-
|
213-
p = any(MicrosoftAspNetCoreMvcController c).getAnActionMethod().getAParameter()
214-
)
215-
}
216-
217-
override string getSourceType() { result = "ASP.NET Core MVC action method parameter" }
218-
}
7+
import semmle.code.csharp.security.dataflow.flowsources.Remote

0 commit comments

Comments
 (0)