FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

C#: Restrict `FormatInvalid.ql` and `UncontrolledFormatString.ql` to … · github/codeql@d53faa8 · GitHub

/ codeql Public

Commit d53faa8

Browse files
hvitved
committed
C#: Restrict FormatInvalid.ql and UncontrolledFormatString.ql to calls with insertions
1 parent 7a13214 commit d53faa8

9 files changed

Lines changed: 18 additions & 7 deletions

File tree

‎csharp/ql/src/API Abuse/FormatInvalid.ql‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,7 @@ import semmle.code.csharp.frameworks.Format
1414
import FormatFlow
1515

1616
from FormatCall s, InvalidFormatString src, PathNode source, PathNode sink
17-
where hasFlowPath(src, source, s, sink)
17+
where
18+
hasFlowPath(src, source, s, sink) and
19+
s.hasInsertions()
1820
select src, source, sink, "Invalid format string used in $@ formatting call.", s, "this"

‎csharp/ql/src/API Abuse/FormatInvalidBad.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ class Bad
44
{
55
string GenerateEmptyClass(string c)
66
{
7-
return string.Format("class {0} { }");
7+
return string.Format("class {0} { }", "C");
88
}
99
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class FormatStringConfiguration extends TaintTracking::Configuration {
2727
}
2828

2929
override predicate isSink(DataFlow::Node sink) {
30-
sink.asExpr() = any(FormatCall call).getFormatExpr()
30+
sink.asExpr() = any(FormatCall call | call.hasInsertions()).getFormatExpr()
3131
}
3232
}
3333

‎csharp/ql/src/semmle/code/csharp/frameworks/Format.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@ class FormatCall extends MethodCall {
218218
/** Gets the argument number of the first supplied insert. */
219219
int getFirstArgument() { result = this.getFormatArgument() + 1 }
220220

221+
/** Holds if this call has one or more insertions. */
222+
predicate hasInsertions() { exists(this.getArgument(this.getFirstArgument())) }
223+
221224
/** Holds if the arguments are supplied in an array, not individually. */
222225
predicate hasArrayExpr() {
223226
this.getNumberOfArguments() = this.getFirstArgument() + 1 and

‎csharp/ql/test/query-tests/API Abuse/FormatInvalid/FormatInvalid.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ void FormatMethodTests()
115115
System.Diagnostics.Debug.Assert(true, "Error", "}", ps);
116116
sw.Write("}", 0);
117117
System.Diagnostics.Debug.Print("}", ps);
118+
119+
Console.WriteLine("}"); // GOOD
118120
}
119121

120122
System.IO.StringWriter sw;

‎csharp/ql/test/query-tests/API Abuse/FormatInvalid/FormatInvalid.expected‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ nodes
5151
| FormatInvalid.cs:115:56:115:58 | [assertion success] "}" | semmle.label | [assertion success] "}" |
5252
| FormatInvalid.cs:116:18:116:20 | "}" | semmle.label | "}" |
5353
| FormatInvalid.cs:117:40:117:42 | "}" | semmle.label | "}" |
54+
| FormatInvalid.cs:119:27:119:29 | "}" | semmle.label | "}" |
5455
| FormatInvalidBad.cs:7:30:7:44 | "class {0} { }" | semmle.label | "class {0} { }" |
5556
| FormatInvalidGood.cs:7:30:7:46 | "class {0} {{ }}" | semmle.label | "class {0} {{ }}" |
5657
edges
@@ -96,4 +97,4 @@ edges
9697
| FormatInvalid.cs:115:57:115:58 | "}" | FormatInvalid.cs:115:56:115:58 | [assertion success] "}" | FormatInvalid.cs:115:56:115:58 | [assertion success] "}" | Invalid format string used in $@ formatting call. | FormatInvalid.cs:115:9:115:63 | call to method Assert | this |
9798
| FormatInvalid.cs:116:19:116:20 | "}" | FormatInvalid.cs:116:18:116:20 | "}" | FormatInvalid.cs:116:18:116:20 | "}" | Invalid format string used in $@ formatting call. | FormatInvalid.cs:116:9:116:24 | call to method Write | this |
9899
| FormatInvalid.cs:117:41:117:42 | "}" | FormatInvalid.cs:117:40:117:42 | "}" | FormatInvalid.cs:117:40:117:42 | "}" | Invalid format string used in $@ formatting call. | FormatInvalid.cs:117:9:117:47 | call to method Print | this |
99-
| FormatInvalidBad.cs:7:41:7:44 | "class {0} { }" | FormatInvalidBad.cs:7:30:7:44 | "class {0} { }" | FormatInvalidBad.cs:7:30:7:44 | "class {0} { }" | Invalid format string used in $@ formatting call. | FormatInvalidBad.cs:7:16:7:45 | call to method Format | this |
100+
| FormatInvalidBad.cs:7:41:7:44 | "class {0} { }" | FormatInvalidBad.cs:7:30:7:44 | "class {0} { }" | FormatInvalidBad.cs:7:30:7:44 | "class {0} { }" | Invalid format string used in $@ formatting call. | FormatInvalidBad.cs:7:16:7:50 | call to method Format | this |

‎csharp/ql/test/query-tests/API Abuse/FormatInvalid/FormatInvalidBad.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ class Bad
44
{
55
string GenerateEmptyClass(string c)
66
{
7-
return string.Format("class {0} { }");
7+
return string.Format("class {0} { }", "C");
88
}
99
}

‎csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatString.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ public void ProcessRequest(HttpContext ctx)
2121

2222
// GOOD: Not the format string.
2323
String.Format((IFormatProvider)null, "Do not do this", path);
24+
25+
// GOOD: Not a formatting call
26+
Console.WriteLine(path);
2427
}
2528

2629
System.Windows.Forms.TextBox box1;

‎csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatString.expected‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ nodes
66
| UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
77
| UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | semmle.label | access to local variable path |
88
| UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | semmle.label | access to local variable path |
9-
| UncontrolledFormatString.cs:31:23:31:31 | access to property Text | semmle.label | access to property Text |
9+
| UncontrolledFormatString.cs:34:23:34:31 | access to property Text | semmle.label | access to property Text |
1010
| UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
1111
| UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | semmle.label | access to local variable format |
1212
#select
1313
| UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | access to property QueryString |
1414
| UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | access to property QueryString |
15-
| UncontrolledFormatString.cs:31:23:31:31 | access to property Text | UncontrolledFormatString.cs:31:23:31:31 | access to property Text | UncontrolledFormatString.cs:31:23:31:31 | access to property Text | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:31:23:31:31 | access to property Text | access to property Text |
15+
| UncontrolledFormatString.cs:34:23:34:31 | access to property Text | UncontrolledFormatString.cs:34:23:34:31 | access to property Text | UncontrolledFormatString.cs:34:23:34:31 | access to property Text | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:34:23:34:31 | access to property Text | access to property Text |
1616
| UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | $@ flows to here and is used as a format string. | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString | access to property QueryString |

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL