Skip to content

Commit 7f25efd

Browse files
hvitvedhvitved
authored
Merge pull request #4858 from hvitved/csharp/merge-format-queries
C#: Merge queries `FormatInvalid.ql`, `FormatMissingArgument.ql`, and `FormatUnusedArgument.ql`
2 parents 8619422 + 1237e56 commit 7f25efd

26 files changed

+264
-267
lines changed

‎csharp/change-notes/2020-12-21-merge-format-queries.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* The queries `FormatInvalid.ql`, `FormatMissingArgument.ql`, and `FormatUnusedArgument.ql` have been merged into a single `FormatInvalid.ql` query.

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

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,46 @@
44
<qhelp>
55
<overview>
66
<p>
7-
The format string supplied to formatting methods (such as <code>string.Format()</code>)
8-
must be formatted correctly, otherwise the exception <code>System.FormatException</code>
9-
will be thrown.
7+
When using string formatting methods (such as <code>string.Format()</code>), the following
8+
should be taken into account:
109
</p>
10+
<ol>
11+
<li>
12+
The formatting string must be formatted correctly, otherwise the exception
13+
<code>System.FormatException</code> will be thrown.
14+
</li>
15+
<li>
16+
All passed arguments should be used by the formatting string, otherwise such
17+
arguments will be ignored.
18+
</li>
19+
<li>
20+
Missing arguments will result in a <code>System.FormatException</code> exception
21+
being thrown.
22+
</li>
23+
</ol>
1124

1225
</overview>
1326

1427
<recommendation>
1528

16-
<p>
17-
Change the format string so that it is correctly formatted. Ensure that each
18-
format item adheres to the syntax:
19-
</p>
20-
21-
<blockquote>
22-
<p><b>{</b><i>index</i>[<b>,</b><i>alignment</i>][<b>:</b><i>formatString</i>]<b>}</b></p>
23-
</blockquote>
24-
25-
<p>
26-
When literals <code>{</code> or <code>}</code> are required, replace them with <code>{{</code> and
27-
<code>}}</code>, respectively, or supply them as arguments.
28-
</p>
29+
<ol>
30+
<li>
31+
Change the format string so that it is correctly formatted. Ensure that each
32+
format item adheres to the syntax:
33+
<blockquote>
34+
<p><b>{</b><i>index</i>[<b>,</b><i>alignment</i>][<b>:</b><i>formatString</i>]<b>}</b></p>
35+
</blockquote>
36+
When literals <code>{</code> or <code>}</code> are required, replace them with <code>{{</code> and
37+
<code>}}</code>, respectively, or supply them as arguments.
38+
</li>
39+
<li>
40+
Change the format string to use the highlighted argument, or remove the unnecessary argument.
41+
</li>
42+
<li>
43+
Supply the correct number of arguments to the format method, or change the format string
44+
to use the correct arguments.
45+
</li>
46+
</ol>
2947

3048
</recommendation>
3149

@@ -40,8 +58,39 @@ literals are not properly escaped.
4058
In the revised example, the literals are properly escaped.
4159
</p>
4260
<sample src="FormatInvalidGood.cs" />
61+
</example>
4362

63+
<example>
64+
<p>
65+
Here are three examples where the format string does not use all the arguments.
66+
</p>
67+
<sample src="FormatUnusedArgumentBad.cs"/>
68+
<ul>
69+
<li>On line 7, the second argument (<code>ex.HResult</code>) is not logged.</li>
70+
<li>On line 8, the first argument (<code>ex</code>) is not logged but the second
71+
argument (<code>ex.HResult</code>) is logged twice.</li>
72+
<li>On line 9, a C-style format string is used, which is incorrect, and neither
73+
argument will be logged.</li>
74+
</ul>
4475
</example>
76+
77+
<example>
78+
<p>
79+
Here are two examples where the call to <code>String.Format()</code> is missing arguments.
80+
</p>
81+
<sample src="FormatMissingArgumentBad.cs"/>
82+
<ul>
83+
<li>On line 7, the second argument (<code>last</code>) is not supplied.</li>
84+
<li>On line 8, the format items are numbered <code>{1}</code> and <code>{2}</code>,
85+
instead of <code>{0}</code> and <code>{1}</code> as they should be.</li>
86+
</ul>
87+
88+
<p>
89+
In the revised example, both arguments are supplied.
90+
</p>
91+
<sample src="FormatMissingArgumentGood.cs"/>
92+
</example>
93+
4594
<references>
4695
<li>MSDN: <a href="https://msdn.microsoft.com/en-us/library/system.string.format.aspx">String.Format Method</a>.</li>
4796
<li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/standard/base-types/composite-formatting">Composite Formatting</a>.</li>

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

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,86 @@
11
/**
2-
* @name Invalid format string
3-
* @description Using a format string with an incorrect format causes a 'System.FormatException'.
2+
* @name Invalid string formatting
3+
* @description Calling 'string.Format()' with either an invalid format string or incorrect
4+
* number of arguments may result in dropped arguments or a 'System.FormatException'.
45
* @kind path-problem
56
* @problem.severity error
67
* @precision high
7-
* @id cs/invalid-format-string
8+
* @id cs/invalid-string-formatting
89
* @tags reliability
910
* maintainability
1011
*/
1112

1213
import csharp
1314
import semmle.code.csharp.frameworks.Format
14-
import FormatFlow
15+
import DataFlow::PathGraph
1516

16-
from FormatCall s, InvalidFormatString src, PathNode source, PathNode sink
17+
private class FormatConfiguration extends DataFlow::Configuration {
18+
FormatConfiguration() { this = "format" }
19+
20+
override predicate isSource(DataFlow::Node n) { n.asExpr() instanceof StringLiteral }
21+
22+
override predicate isSink(DataFlow::Node n) {
23+
exists(FormatCall c | n.asExpr() = c.getFormatExpr())
24+
}
25+
}
26+
27+
private predicate invalidFormatString(
28+
InvalidFormatString src, DataFlow::PathNode source, DataFlow::PathNode sink, string msg,
29+
FormatCall call, string callString
30+
) {
31+
source.getNode().asExpr() = src and
32+
sink.getNode().asExpr() = call.getFormatExpr() and
33+
any(FormatConfiguration conf).hasFlowPath(source, sink) and
34+
call.hasInsertions() and
35+
msg = "Invalid format string used in $@ formatting call." and
36+
callString = "this"
37+
}
38+
39+
private predicate unusedArgument(
40+
FormatCall call, DataFlow::PathNode source, DataFlow::PathNode sink, string msg,
41+
ValidFormatString src, string srcString, Expr unusedExpr, string unusedString
42+
) {
43+
exists(int unused |
44+
source.getNode().asExpr() = src and
45+
sink.getNode().asExpr() = call.getFormatExpr() and
46+
any(FormatConfiguration conf).hasFlowPath(source, sink) and
47+
unused = call.getASuppliedArgument() and
48+
not unused = src.getAnInsert() and
49+
not src.getValue() = "" and
50+
msg = "The $@ ignores $@." and
51+
srcString = "format string" and
52+
unusedExpr = call.getSuppliedExpr(unused) and
53+
unusedString = "this supplied value"
54+
)
55+
}
56+
57+
private predicate missingArgument(
58+
FormatCall call, DataFlow::PathNode source, DataFlow::PathNode sink, string msg,
59+
ValidFormatString src, string srcString
60+
) {
61+
exists(int used, int supplied |
62+
source.getNode().asExpr() = src and
63+
sink.getNode().asExpr() = call.getFormatExpr() and
64+
any(FormatConfiguration conf).hasFlowPath(source, sink) and
65+
used = src.getAnInsert() and
66+
supplied = call.getSuppliedArguments() and
67+
used >= supplied and
68+
msg = "Argument '{" + used + "}' has not been supplied to $@ format string." and
69+
srcString = "this"
70+
)
71+
}
72+
73+
from
74+
Element alert, DataFlow::PathNode source, DataFlow::PathNode sink, string msg, Element extra1,
75+
string extra1String, Element extra2, string extra2String
1776
where
18-
hasFlowPath(src, source, s, sink) and
19-
s.hasInsertions()
20-
select src, source, sink, "Invalid format string used in $@ formatting call.", s, "this"
77+
invalidFormatString(alert, source, sink, msg, extra1, extra1String) and
78+
extra2 = extra1 and
79+
extra2String = extra1String
80+
or
81+
unusedArgument(alert, source, sink, msg, extra1, extra1String, extra2, extra2String)
82+
or
83+
missingArgument(alert, source, sink, msg, extra1, extra1String) and
84+
extra2 = extra1 and
85+
extra2String = extra1String
86+
select alert, source, sink, msg, extra1, extra1String, extra2, extra2String

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
using System;
22

3-
class Bad
3+
class Bad1
44
{
55
string GenerateEmptyClass(string c)
66
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
using System;
22

3-
class Good
3+
class Good1
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/API Abuse/FormatMissingArgument.qhelp‎

Lines changed: 0 additions & 41 deletions
This file was deleted.

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

Lines changed: 0 additions & 24 deletions
This file was deleted.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
using System;
22

3-
class Bad
3+
class Bad3
44
{
55
void Hello(string first, string last)
66
{

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
using System;
22

3-
class Good
3+
class Good3
44
{
55
void Hello(string first, string last)
66
{

‎csharp/ql/src/API Abuse/FormatUnusedArgument.qhelp‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

0 commit comments

Comments
 (0)