Skip to content

Commit c9ffb38

Browse files
calumgrantcalumgrant
committed
C#: Add sources and sinks in Winforms. Update some queries with new sources and sinks.
1 parent f85f05d commit c9ffb38

File tree

21 files changed

+271
-13
lines changed

21 files changed

+271
-13
lines changed

‎csharp/ql/src/Security Features/CWE-089/SqlInjection.ql‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@ import csharp
1414
import semmle.code.csharp.security.dataflow.SqlInjection::SqlInjection
1515
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
1616

17+
string getSourceType(DataFlow::Node node) {
18+
result = node.(RemoteFlowSource).getSourceType()
19+
or
20+
result = node.(LocalFlowSource).getSourceType()
21+
}
22+
1723
from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
1824
where c.hasFlowPath(source, sink)
1925
select sink.getNode(), source, sink, "Query might include code from $@.", source,
20-
("this " + source.getNode().(RemoteFlowSource).getSourceType())
26+
("this " + getSourceType(source.getNode()))

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,19 @@
1212

1313
import csharp
1414
import semmle.code.csharp.dataflow.flowsources.Remote
15+
import semmle.code.csharp.dataflow.flowsources.Local
1516
import semmle.code.csharp.dataflow.TaintTracking
1617
import semmle.code.csharp.frameworks.Format
1718
import DataFlow::PathGraph
1819

1920
class FormatStringConfiguration extends TaintTracking::Configuration {
2021
FormatStringConfiguration() { this = "FormatStringConfiguration" }
2122

22-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
23+
override predicate isSource(DataFlow::Node source) {
24+
source instanceof RemoteFlowSource
25+
or
26+
source instanceof LocalFlowSource
27+
}
2328

2429
override predicate isSink(DataFlow::Node sink) {
2530
sink.asExpr() = any(FormatCall call).getFormatExpr()

‎csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Local.qll‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Provides classes representing sources of local input.
3+
*/
4+
5+
import csharp
6+
private import semmle.code.csharp.frameworks.system.windows.Forms
7+
8+
/** A data flow source of local data. */
9+
abstract class LocalFlowSource extends DataFlow::Node {
10+
/** Gets a string that describes the type of this local flow source. */
11+
abstract string getSourceType();
12+
}
13+
14+
/** A data flow source of local user input. */
15+
abstract class LocalUserInputSource extends LocalFlowSource { }
16+
17+
/** The text of a `TextBox`. */
18+
class TextFieldSource extends LocalUserInputSource {
19+
TextFieldSource() {
20+
this.asExpr() = any(TextControl control).getARead()
21+
}
22+
23+
override string getSourceType() { result = "TextBox text" }
24+
}

‎csharp/ql/src/semmle/code/csharp/frameworks/system/windows/Forms.qll‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,78 @@ class SystemWindowsFormsHtmlElement extends SystemWindowsFormsClass {
2323
/** Gets the `SetAttribute` method. */
2424
Method getSetAttributeMethod() { result = this.getAMethod("SetAttribute") }
2525
}
26+
27+
/** The `System.Windows.Forms.TextBoxBase` class. */
28+
class SystemWindowsFormsTextBoxBase extends SystemWindowsFormsClass {
29+
SystemWindowsFormsTextBoxBase() {
30+
this.hasName("TextBoxBase")
31+
}
32+
33+
/** Gets the `Text` property. */
34+
Property getTextProperty() { result = this.getProperty("Text") }
35+
}
36+
37+
/** The `System.Windows.Forms.RichTextBox` class. */
38+
class SystemWindowsFormsRichTextBox extends SystemWindowsFormsClass {
39+
SystemWindowsFormsRichTextBox() {
40+
this.hasName("RichTextBox")
41+
}
42+
43+
/** Gets the `Rtf` property. */
44+
Property getRtfProperty() { result = this.getProperty("Rtf") }
45+
46+
/** Gets the `SelectedText` property. */
47+
Property getSelectedTextProperty() { result = this.getProperty("SelectedText") }
48+
49+
/** Gets the 'SelectedRtf' property. */
50+
Property getSelectedRtfProperty() { result = this.getProperty("SelectedRtf") }
51+
}
52+
53+
/** The `System.Windows.Forms.HtmlDocument` class. */
54+
class SystemWindowsFormsHtmlDocumentClass extends SystemWindowsFormsClass {
55+
SystemWindowsFormsHtmlDocumentClass() {
56+
this.hasName("HtmlDocument")
57+
}
58+
59+
/** Gets the `Write` method. */
60+
Method getWriteMethod() { result = this.getAMethod() and result.hasName("Write") }
61+
}
62+
63+
/** The `System.Windows.Forms.WebBrowser` class. */
64+
class SystemWindowsFormsWebBrowserClass extends SystemWindowsFormsClass {
65+
SystemWindowsFormsWebBrowserClass() {
66+
this.hasName("WebBrowser")
67+
}
68+
69+
/** Gets the `DocumentText` property. */
70+
Property getDocumentTextProperty() { result = this.getProperty("DocumentText") }
71+
}
72+
73+
private class TextProperty extends Property {
74+
TextProperty() {
75+
exists(SystemWindowsFormsRichTextBox c |
76+
this = c.getRtfProperty() or
77+
this = c.getSelectedTextProperty() or
78+
this = c.getSelectedRtfProperty()
79+
)
80+
or
81+
exists(SystemWindowsFormsTextBoxBase tb |
82+
this = tb.getTextProperty().getAnOverrider*()
83+
)
84+
}
85+
}
86+
87+
/** A field that contains a text control. */
88+
class TextControl extends Field
89+
{
90+
TextControl() {
91+
this.getType().(ValueOrRefType).getBaseClass*() instanceof SystemWindowsFormsTextBoxBase
92+
}
93+
94+
/** Gets a read of the text property. */
95+
PropertyRead getARead() {
96+
result.getTarget() instanceof TextProperty
97+
and
98+
result.getQualifier() = this.getAnAccess()
99+
}
100+
}

‎csharp/ql/src/semmle/code/csharp/security/PrivateData.qll‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111

1212
import csharp
13+
import semmle.code.csharp.frameworks.system.windows.Forms
1314

1415
/** A string for `match` that identifies strings that look like they represent private data. */
1516
private string privateNames() {
@@ -58,3 +59,10 @@ class PrivateVariableAccess extends PrivateDataExpr, VariableAccess {
5859
exists(string s | this.getTarget().getName().toLowerCase() = s | s.matches(privateNames()))
5960
}
6061
}
62+
63+
/** Reading the text property of a control that might contain private data. */
64+
class PrivateControlAccess extends PrivateDataExpr {
65+
PrivateControlAccess() {
66+
exists(TextControl c | this = c.getARead() and c.getName().toLowerCase().matches(privateNames()))
67+
}
68+
}

‎csharp/ql/src/semmle/code/csharp/security/SensitiveActions.qll‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111

1212
import csharp
13+
import semmle.code.csharp.frameworks.system.windows.Forms
1314

1415
/**
1516
* A string for `match` that identifies strings that look like they represent secret data.
@@ -108,7 +109,11 @@ private predicate expressionHasName(Expr expr, string name) {
108109

109110
/** An expression that may contain a password. */
110111
class PasswordExpr extends Expr {
111-
PasswordExpr() { exists(string name | expressionHasName(this, name) and isPassword(name)) }
112+
PasswordExpr() {
113+
exists(string name | expressionHasName(this, name) and isPassword(name))
114+
or
115+
this instanceof PasswordTextboxText
116+
}
112117
}
113118

114119
/** An expression that might contain sensitive data. */
@@ -130,6 +135,26 @@ class SensitiveVariableAccess extends SensitiveExpr, VariableAccess {
130135
SensitiveVariableAccess() { isSuspicious(this.getTarget().getName()) }
131136
}
132137

138+
/** Reading the `Text` property of a password text box. */
139+
class PasswordTextboxText extends SensitiveExpr, PropertyRead {
140+
PasswordTextboxText() {
141+
this = any(PasswordField p).getARead()
142+
}
143+
}
144+
145+
/** A field containing a text box used as a password. */
146+
class PasswordField extends TextControl
147+
{
148+
PasswordField() {
149+
isSuspicious(this.getName())
150+
or
151+
exists(PropertyWrite write | write.getQualifier() = this.getAnAccess() |
152+
write.getTarget().getName() = "UseSystemPasswordChar" or
153+
write.getTarget().getName() = "PasswordChar"
154+
)
155+
}
156+
}
157+
133158
/** A method that may produce sensitive data. */
134159
abstract class SensitiveDataMethod extends Method { }
135160

‎csharp/ql/src/semmle/code/csharp/security/dataflow/CodeInjection.qll‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import csharp
66

77
module CodeInjection {
88
import semmle.code.csharp.dataflow.flowsources.Remote
9+
import semmle.code.csharp.dataflow.flowsources.Local
910
import semmle.code.csharp.frameworks.system.codedom.Compiler
1011
import semmle.code.csharp.security.Sanitizers
1112

@@ -40,6 +41,9 @@ module CodeInjection {
4041
/** A source of remote user input. */
4142
class RemoteSource extends Source { RemoteSource() { this instanceof RemoteFlowSource } }
4243

44+
/** A source of local user input. */
45+
class LocalSource extends Source { LocalSource() { this instanceof LocalFlowSource } }
46+
4347
private class SimpleTypeSanitizer extends Sanitizer, SimpleTypeSanitizedExpr { }
4448

4549
private class GuidSanitizer extends Sanitizer, GuidSanitizedExpr { }

‎csharp/ql/src/semmle/code/csharp/security/dataflow/ResourceInjection.qll‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import csharp
66

77
module ResourceInjection {
88
import semmle.code.csharp.dataflow.flowsources.Remote
9+
import semmle.code.csharp.dataflow.flowsources.Local
910
import semmle.code.csharp.frameworks.system.Data
1011
import semmle.code.csharp.security.Sanitizers
1112

@@ -40,6 +41,9 @@ module ResourceInjection {
4041
/** A source of remote user input. */
4142
class RemoteSource extends Source { RemoteSource() { this instanceof RemoteFlowSource } }
4243

44+
/** A source of local user input. */
45+
class LocalSource extends Source { LocalSource() { this instanceof LocalFlowSource } }
46+
4347
/** An argument to the `ConnectionString` property on a data connection class. */
4448
class SqlConnectionStringSink extends Sink {
4549
SqlConnectionStringSink() {

‎csharp/ql/src/semmle/code/csharp/security/dataflow/SqlInjection.qll‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import csharp
66

77
module SqlInjection {
88
import semmle.code.csharp.dataflow.flowsources.Remote
9+
import semmle.code.csharp.dataflow.flowsources.Local
910
import semmle.code.csharp.frameworks.Sql
1011
import semmle.code.csharp.security.Sanitizers
1112

@@ -40,6 +41,9 @@ module SqlInjection {
4041
/** A source of remote user input. */
4142
class RemoteSource extends Source { RemoteSource() { this instanceof RemoteFlowSource } }
4243

44+
/** A source of local user input. */
45+
class LocalSource extends Source { LocalSource() { this instanceof LocalFlowSource } }
46+
4347
/** An SQL expression passed to an API call that executes SQL. */
4448
class SqlInjectionExprSink extends Sink {
4549
SqlInjectionExprSink() { exists(SqlExpr s | this.getExpr() = s.getSql()) }

‎csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -572,9 +572,7 @@ module XSS {
572572
}
573573
}
574574

575-
/**
576-
* HtmlString that may be rendered as is need to have sanitized value
577-
*/
575+
/** `HtmlString` that may be rendered as is need to have sanitized value. */
578576
class MicrosoftAspNetHtmlStringSink extends AspNetCoreSink {
579577
MicrosoftAspNetHtmlStringSink() {
580578
exists(ObjectCreation c, MicrosoftAspNetCoreHttpHtmlString s |

0 commit comments

Comments
 (0)