Skip to content

Commit 5467718

Browse files
hvitvedhvitved
committed
C#: Introduce RemoteFlowSink class
1 parent 142737d commit 5467718

File tree

9 files changed

+56
-17
lines changed

9 files changed

+56
-17
lines changed

‎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/semmle/code/csharp/security/dataflow/XSS.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ module XSS {
1212
import semmle.code.csharp.frameworks.system.web.UI
1313
import semmle.code.csharp.security.Sanitizers
1414
import semmle.code.csharp.security.dataflow.flowsinks.Html
15+
import semmle.code.csharp.security.dataflow.flowsinks.Remote
1516
import semmle.code.csharp.security.dataflow.flowsources.Remote
1617

1718
/**
@@ -108,8 +109,11 @@ module XSS {
108109

109110
/**
110111
* A data flow sink for cross-site scripting (XSS) vulnerabilities.
112+
*
113+
* Any XSS sink is also a remote flow sink, so this class contributes
114+
* to the abstract class `RemoteFlowSink`.
111115
*/
112-
abstract class Sink extends DataFlow::ExprNode {
116+
abstract class Sink extends DataFlow::ExprNode, RemoteFlowSink {
113117
string explanation() { none() }
114118
}
115119

‎csharp/ql/src/semmle/code/csharp/security/dataflow/Email.qll renamed to ‎csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Email.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
/** Provides data flow sinks for sending email. */
22

33
import csharp
4+
private import Remote
45
private import semmle.code.csharp.frameworks.system.net.Mail
56

67
module Email {
78
/** A data flow sink for sending email. */
8-
abstract class Sink extends DataFlow::ExprNode { }
9+
abstract class Sink extends DataFlow::ExprNode, RemoteFlowSink { }
910

1011
/** A data flow sink for sending email via `System.Net.Mail.MailMessage`. */
1112
class MailMessageSink extends Sink {

‎csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import csharp
6+
private import Remote
67
private import semmle.code.csharp.commons.Loggers
78
private import semmle.code.csharp.frameworks.system.Web
89

@@ -45,7 +46,7 @@ class TraceMessageSink extends ExternalLocationSink {
4546
/**
4647
* An expression set as a value on a cookie instance.
4748
*/
48-
class CookieStorageSink extends ExternalLocationSink {
49+
class CookieStorageSink extends ExternalLocationSink, RemoteFlowSink {
4950
CookieStorageSink() {
5051
exists(Expr e | e = this.getExpr() |
5152
e = any(SystemWebHttpCookie cookie).getAConstructor().getACall().getArgumentForName("value")

‎csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Html.qll

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import csharp
6+
private import Remote
67
private import semmle.code.csharp.frameworks.microsoft.AspNetCore
78
private import semmle.code.csharp.frameworks.system.Net
89
private import semmle.code.csharp.frameworks.system.Web
@@ -18,7 +19,7 @@ private import semmle.code.asp.AspNet
1819
* A sink where the value of the expression may be rendered as HTML,
1920
* without implicit HTML encoding.
2021
*/
21-
abstract class HtmlSink extends DataFlow::ExprNode { }
22+
abstract class HtmlSink extends DataFlow::ExprNode, RemoteFlowSink { }
2223

2324
/**
2425
* An expression that is used as an argument to an HTML sink method on
@@ -101,7 +102,7 @@ class SystemWebSetterHtmlSink extends HtmlSink {
101102
exists(Property p, string name, ValueOrRefType declaringType |
102103
declaringType = p.getDeclaringType() and
103104
any(SystemWebUINamespace n).getAChildNamespace*() = declaringType.getNamespace() and
104-
this.getExpr() = p.getSetter().getParameter(0).getAnAssignedArgument() and
105+
this.getExpr() = p.getAnAssignedValue() and
105106
p.hasName(name)
106107
|
107108
name = "Caption" and

‎csharp/ql/src/semmle/code/csharp/security/dataflow/flowsinks/Remote.qll

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* Provides classes representing data flow sinks for remote user output.
3+
*/
4+
5+
import csharp
6+
private import Email
7+
private import ExternalLocationSink
8+
private import Html
9+
private import semmle.code.csharp.security.dataflow.XSS
10+
private import semmle.code.csharp.frameworks.system.web.UI
11+
12+
/** A data flow sink of remote user output. */
13+
abstract class RemoteFlowSink extends DataFlow::Node { }
14+
15+
/**
16+
* A value written to the `[Inner]Text` property of an object defined in the
17+
* `System.Web.UI` namespace.
18+
*/
19+
class SystemWebUIText extends RemoteFlowSink {
20+
SystemWebUIText() {
21+
exists(Property p, string name |
22+
p.getDeclaringType().getNamespace().getParentNamespace*() instanceof SystemWebUINamespace and
23+
this.asExpr() = p.getAnAssignedValue() and
24+
p.hasName(name)
25+
|
26+
name = "Text"
27+
or
28+
name = "InnerText"
29+
)
30+
}
31+
}

‎csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ public void ProcessRequest(HttpContext ctx)
3838
log("Exception occurred", ex);
3939
ctx.Response.Write("Exception occurred");
4040

41-
textBox.Text = ex.InnerException.StackTrace; // BAD (false negative)
42-
textBox.Text = ex.StackTrace; // BAD (false negative)
43-
textBox.Text = ex.ToString(); // BAD (false negative)
41+
textBox.Text = ex.InnerException.StackTrace; // BAD
42+
textBox.Text = ex.StackTrace; // BAD
43+
textBox.Text = ex.ToString(); // BAD
4444
textBox.Text = ex.Message; // GOOD
4545
return;
4646
}

‎csharp/ql/test/query-tests/Security Features/CWE-209/ExceptionInformationExposure.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,16 @@ nodes
55
| ExceptionInformationExposure.cs:21:32:21:44 | call to method ToString | semmle.label | call to method ToString |
66
| ExceptionInformationExposure.cs:23:32:23:33 | access to local variable ex | semmle.label | access to local variable ex |
77
| ExceptionInformationExposure.cs:25:32:25:44 | access to property StackTrace | semmle.label | access to property StackTrace |
8+
| ExceptionInformationExposure.cs:41:28:41:55 | access to property StackTrace | semmle.label | access to property StackTrace |
9+
| ExceptionInformationExposure.cs:42:28:42:40 | access to property StackTrace | semmle.label | access to property StackTrace |
10+
| ExceptionInformationExposure.cs:43:28:43:40 | call to method ToString | semmle.label | call to method ToString |
811
| ExceptionInformationExposure.cs:49:28:49:55 | call to method ToString | semmle.label | call to method ToString |
912
#select
1013
| 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 |
1114
| 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 |
1215
| 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 |
1316
| 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 |
17+
| 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 |
18+
| 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 |
19+
| 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 |
1420
| 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 |

0 commit comments

Comments
 (0)