Skip to content

Commit f760731

Browse files
committed
CPP: Fix FPs.
1 parent 9a407eb commit f760731

File tree

7 files changed

+26
-14
lines changed

7 files changed

+26
-14
lines changed

cpp/ql/src/semmle/code/cpp/commons/Printf.qll

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,12 @@ class FormattingFunctionCall extends Expr {
169169
* Gets the number of arguments to this call that are parameters to the
170170
* format string.
171171
*/
172-
int getNumFormatArgument() { result = count(this.getFormatArgument(_)) }
172+
int getNumFormatArgument() {
173+
result = count(this.getFormatArgument(_)) and
174+
175+
// format arguments must be known
176+
exists(getTarget().(FormattingFunction).getFirstFormatArgumentIndex())
177+
}
173178
}
174179

175180
/**

cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,20 @@ abstract class FormattingFunction extends Function {
115115
* Gets the position of the first format argument, corresponding with
116116
* the first format specifier in the format string.
117117
*/
118-
int getFirstFormatArgumentIndex() { result = getNumberOfParameters() }
118+
int getFirstFormatArgumentIndex() {
119+
result = getNumberOfParameters()
120+
and
121+
// the formatting function either has a definition in the snapshot, or all
122+
// `DeclarationEntry`s agree on the number of parameters (otherwise we don't
123+
// really know the correct number)
124+
(
125+
hasDefinition() or
126+
forall(FunctionDeclarationEntry fde |
127+
fde = getADeclarationEntry() |
128+
result = fde.getNumberOfParameters()
129+
)
130+
)
131+
}
119132

120133
/**
121134
* Gets the position of the buffer size argument, if any.

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
| a.c:14:3:14:25 | call to myMultiplyDefinedPrintf | Format expects 1 arguments but given 2 |
2-
| a.c:17:3:17:26 | call to myMultiplyDefinedPrintf2 | Format expects 1 arguments but given 2 |
32
| b.c:11:3:11:25 | call to myMultiplyDefinedPrintf | Format expects 1 arguments but given 2 |
4-
| b.c:14:3:14:26 | call to myMultiplyDefinedPrintf2 | Format expects 1 arguments but given 2 |
53
| c.c:7:3:7:25 | call to myMultiplyDefinedPrintf | Format expects 1 arguments but given 2 |
6-
| c.c:10:3:10:26 | call to myMultiplyDefinedPrintf2 | Format expects 1 arguments but given 2 |
74
| custom_printf.cpp:31:5:31:12 | call to myPrintf | Format expects 2 arguments but given 3 |
85
| macros.cpp:12:2:12:31 | call to printf | Format expects 2 arguments but given 3 |
96
| macros.cpp:16:2:16:30 | call to printf | Format expects 2 arguments but given 3 |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
| a.c:12:3:12:25 | call to myMultiplyDefinedPrintf | Format expects 1 arguments but given 0 |
2-
| a.c:15:3:15:26 | call to myMultiplyDefinedPrintf2 | Format expects 1 arguments but given 0 |
32
| b.c:9:3:9:25 | call to myMultiplyDefinedPrintf | Format expects 1 arguments but given 0 |
4-
| b.c:12:3:12:26 | call to myMultiplyDefinedPrintf2 | Format expects 1 arguments but given 0 |
53
| c.c:5:3:5:25 | call to myMultiplyDefinedPrintf | Format expects 1 arguments but given 0 |
6-
| c.c:8:3:8:26 | call to myMultiplyDefinedPrintf2 | Format expects 1 arguments but given 0 |
74
| custom_printf.cpp:29:5:29:12 | call to myPrintf | Format expects 2 arguments but given 1 |
85
| macros.cpp:14:2:14:37 | call to printf | Format expects 4 arguments but given 3 |
96
| macros.cpp:21:2:21:36 | call to printf | Format expects 4 arguments but given 3 |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/a.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ void test_custom_printf1()
1212
myMultiplyDefinedPrintf("%i", 0); // BAD (too few format arguments)
1313
myMultiplyDefinedPrintf("%i", 0, 1); // GOOD
1414
myMultiplyDefinedPrintf("%i", 0, 1, 2); // BAD (too many format arguments)
15-
myMultiplyDefinedPrintf2("%i", 0); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
15+
myMultiplyDefinedPrintf2("%i", 0); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
1616
myMultiplyDefinedPrintf2("%i", 0, 1); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
17-
myMultiplyDefinedPrintf2("%i", 0, 1, 2); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
17+
myMultiplyDefinedPrintf2("%i", 0, 1, 2); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
1818
}

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/b.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ void test_custom_printf2()
99
myMultiplyDefinedPrintf("%i", 0); // BAD (too few format arguments)
1010
myMultiplyDefinedPrintf("%i", 0, 1); // GOOD
1111
myMultiplyDefinedPrintf("%i", 0, 1, 2); // BAD (too many format arguments)
12-
myMultiplyDefinedPrintf2("%i", 0); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
12+
myMultiplyDefinedPrintf2("%i", 0); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
1313
myMultiplyDefinedPrintf2("%i", 0, 1); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
14-
myMultiplyDefinedPrintf2("%i", 0, 1, 2); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
14+
myMultiplyDefinedPrintf2("%i", 0, 1, 2); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
1515
}

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/c.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ void test_custom_printf2()
55
myMultiplyDefinedPrintf("%i", 0); // BAD (too few format arguments)
66
myMultiplyDefinedPrintf("%i", 0, 1); // GOOD
77
myMultiplyDefinedPrintf("%i", 0, 1, 2); // BAD (too many format arguments)
8-
myMultiplyDefinedPrintf2("%i", 0); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
8+
myMultiplyDefinedPrintf2("%i", 0); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
99
myMultiplyDefinedPrintf2("%i", 0, 1); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
10-
myMultiplyDefinedPrintf2("%i", 0, 1, 2); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
10+
myMultiplyDefinedPrintf2("%i", 0, 1, 2); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
1111
}

0 commit comments

Comments
 (0)