Skip to content

Commit 7dc0a81

Browse files
hvitvedhvitved
authored
Merge pull request #513 from calumgrant/cs/cwe-134
C#: New query cs/uncontrolled-format-string
2 parents c20b688 + 36d9520 commit 7dc0a81

File tree

8 files changed

+149
-1
lines changed

8 files changed

+149
-1
lines changed

‎change-notes/1.19/analysis-csharp.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
| **Query** | **Tags** | **Purpose** |
1212
|-----------------------------|-----------|--------------------------------------------------------------------|
1313
| Using a package with a known vulnerability (cs/use-of-vulnerable-package) | security, external/cwe/cwe-937 | Finds project build files that import packages with known vulnerabilities. This is included by default. |
14-
14+
| Uncontrolled format string (cs/uncontrolled-format-string) | security, external/cwe/cwe-134 | Finds data flow from remote inputs to the format string in `String.Format`. This is included by default. |
1515

1616
## Changes to existing queries
1717

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Passing untrusted format strings to <code>String.Format</code> can throw exceptions
8+
and cause a denial of service. For example, if the format string references a missing argument,
9+
or an argument of the wrong type, then <code>System.FormatException</code> is thrown.
10+
</p>
11+
12+
</overview>
13+
<recommendation>
14+
15+
<p>Use a string literal for the format string to prevent the possibility of data flow from
16+
an untrusted source. This also helps to prevent errors where the arguments to
17+
<code>String.Format</code> do not match the format string.</p>
18+
19+
<p>If the format string cannot be constant, ensure that it comes from a secure
20+
data source or is compiled into the source code.</p>
21+
22+
</recommendation>
23+
<example>
24+
25+
<p>In this example, the format string is read from an HTTP request, which could cause
26+
the application to crash.</p>
27+
28+
<sample src="UncontrolledFormatStringBad.cs" />
29+
30+
</example>
31+
<references>
32+
33+
<li>
34+
OWASP:
35+
<a href="https://www.owasp.org/index.php/Format_string_attack">Format string attack</a>.
36+
</li>
37+
38+
<li>
39+
Microsoft docs:
40+
<a href="https://docs.microsoft.com/en-us/dotnet/api/system.string.format">String.Format Method</a>
41+
</li>
42+
43+
</references>
44+
</qhelp>

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* @name Uncontrolled format string
3+
* @description Passing untrusted format strings from remote data sources can throw exceptions
4+
* and cause a denial of service.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id cs/uncontrolled-format-string
9+
* @tags security
10+
* external/cwe/cwe-134
11+
*/
12+
13+
import csharp
14+
import semmle.code.csharp.dataflow.flowsources.Remote
15+
import semmle.code.csharp.dataflow.TaintTracking
16+
import semmle.code.csharp.frameworks.Format
17+
import DataFlow::PathGraph
18+
19+
class FormatStringConfiguration extends TaintTracking::Configuration {
20+
FormatStringConfiguration() { this = "FormatStringConfiguration" }
21+
22+
override predicate isSource(DataFlow::Node source) {
23+
source instanceof RemoteFlowSource
24+
}
25+
26+
override predicate isSink(DataFlow::Node sink) {
27+
sink.asExpr() = any(FormatCall call).getFormatExpr()
28+
}
29+
}
30+
31+
from FormatStringConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
32+
where config.hasFlowPath(source, sink)
33+
select sink.getNode(), source, sink,
34+
"$@ flows to here and is used as a format string.", source.getNode(), source.getNode().toString()

‎csharp/ql/src/Security Features/CWE-134/UncontrolledFormatStringBad.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
using System.Web;
2+
3+
public class HttpHandler : IHttpHandler
4+
{
5+
string Surname, Forenames, FormattedName;
6+
7+
public void ProcessRequest(HttpContext ctx)
8+
{
9+
string format = ctx.Request.QueryString["nameformat"];
10+
11+
// BAD: Uncontrolled format string.
12+
FormattedName = string.Format(format, Surname, Forenames);
13+
}
14+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// semmle-extractor-options: /r:System.Runtime.Extensions.dll /r:System.Collections.Specialized.dll ${testdir}/../../../resources/stubs/System.Web.cs
2+
3+
using System;
4+
using System.IO;
5+
using System.Web;
6+
7+
public class TaintedPathHandler : IHttpHandler
8+
{
9+
public void ProcessRequest(HttpContext ctx)
10+
{
11+
String path = ctx.Request.QueryString["page"];
12+
13+
// BAD: Uncontrolled format string.
14+
String.Format(path, "Do not do this");
15+
16+
// BAD: Using an IFormatProvider.
17+
String.Format((IFormatProvider)null, path, "Do not do this");
18+
19+
// GOOD: Not the format string.
20+
String.Format("Do not do this", path);
21+
22+
// GOOD: Not the format string.
23+
String.Format((IFormatProvider)null, "Do not do this", path);
24+
}
25+
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
edges
2+
| UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | UncontrolledFormatString.cs:14:23:14:26 | access to local variable path |
3+
| UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | UncontrolledFormatString.cs:17:46:17:49 | access to local variable path |
4+
| UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format |
5+
nodes
6+
| UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString |
7+
| UncontrolledFormatString.cs:14:23:14:26 | access to local variable path |
8+
| UncontrolledFormatString.cs:17:46:17:49 | access to local variable path |
9+
| UncontrolledFormatString.cs:20:23:20:38 | "Do not do this" |
10+
| UncontrolledFormatString.cs:23:46:23:61 | "Do not do this" |
11+
| UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString |
12+
| UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format |
13+
#select
14+
| UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | 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 |
15+
| UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | 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 |
16+
| UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString | 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 |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security Features/CWE-134/UncontrolledFormatString.ql

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
using System.Web;
2+
3+
public class HttpHandler : IHttpHandler
4+
{
5+
string Surname, Forenames, FormattedName;
6+
7+
public void ProcessRequest(HttpContext ctx)
8+
{
9+
string format = ctx.Request.QueryString["nameformat"];
10+
11+
// BAD: Uncontrolled format string.
12+
FormattedName = string.Format(format, Surname, Forenames);
13+
}
14+
}

0 commit comments

Comments
 (0)