Skip to content

Commit cc62683

Browse files
MathiasVPMathiasVP
committed
C++: Fix failing test and accept test cases.
1 parent 834b07e commit cc62683

File tree

2 files changed

+31
-4
lines changed

2 files changed

+31
-4
lines changed

‎cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,14 @@ predicate unboundedWriteSource(Expr e, BufferWrite bw) {
6060

6161
predicate isSource(FS::FlowSource source, string sourceType) { source.getSourceType() = sourceType }
6262

63-
predicate isSink(DataFlow::Node sink, BufferWrite bw) {
64-
unboundedWriteSource(sink.asIndirectExpr(), bw)
65-
or
63+
/**
64+
* Holds if `bw` is a `BufferWrite` that reads from `stdin`. In such cases the
65+
* sink is the outgoing argument that is written to.
66+
*
67+
* By factoring these cases out into this predicate we can place an out barrier
68+
* on exactly these sinks in `Config`.
69+
*/
70+
predicate isSinkFromStdIn(DataFlow::Node sink, BufferWrite bw) {
6671
// `gets` and `scanf` reads from stdin so there's no real input.
6772
// The `BufferWrite` library models this as the call itself being
6873
// the source. In this case we mark the output argument as being
@@ -72,6 +77,12 @@ predicate isSink(DataFlow::Node sink, BufferWrite bw) {
7277
unboundedWriteSource(sink.asDefiningArgument(), bw)
7378
}
7479

80+
predicate isSink(DataFlow::Node sink, BufferWrite bw) {
81+
unboundedWriteSource(sink.asIndirectExpr(), bw)
82+
or
83+
isSinkFromStdIn(sink, bw)
84+
}
85+
7586
predicate lessThanOrEqual(IRGuardCondition g, Expr e, boolean branch) {
7687
exists(Operand left |
7788
g.comparesLt(left, _, _, true, branch) or
@@ -86,7 +97,7 @@ module Config implements DataFlow::ConfigSig {
8697

8798
predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
8899

89-
predicate isBarrierOut(DataFlow::Node node) { isSink(node) }
100+
predicate isBarrierOut(DataFlow::Node node) { isSinkFromStdIn(node, _) }
90101

91102
predicate isBarrier(DataFlow::Node node) {
92103
// Block flow if the node is guarded by any <, <= or = operations.

‎cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,31 @@ edges
22
| main.cpp:6:27:6:30 | argv indirection | main.cpp:10:20:10:23 | argv indirection |
33
| main.cpp:10:20:10:23 | argv indirection | tests.cpp:631:32:631:35 | argv indirection |
44
| tests.cpp:613:19:613:24 | source indirection | tests.cpp:615:17:615:22 | source indirection |
5+
| tests.cpp:622:19:622:24 | source indirection | tests.cpp:625:2:625:16 | ... = ... indirection |
6+
| tests.cpp:625:2:625:16 | ... = ... indirection | tests.cpp:625:4:625:7 | s indirection [post update] [home indirection] |
7+
| tests.cpp:625:4:625:7 | s indirection [post update] [home indirection] | tests.cpp:628:14:628:14 | s indirection [home indirection] |
8+
| tests.cpp:628:14:628:14 | s indirection [home indirection] | tests.cpp:628:14:628:19 | home indirection |
9+
| tests.cpp:628:14:628:14 | s indirection [home indirection] | tests.cpp:628:16:628:19 | home indirection |
10+
| tests.cpp:628:16:628:19 | home indirection | tests.cpp:628:14:628:19 | home indirection |
511
| tests.cpp:631:32:631:35 | argv indirection | tests.cpp:656:9:656:15 | access to array indirection |
12+
| tests.cpp:631:32:631:35 | argv indirection | tests.cpp:657:9:657:15 | access to array indirection |
613
| tests.cpp:656:9:656:15 | access to array indirection | tests.cpp:613:19:613:24 | source indirection |
14+
| tests.cpp:657:9:657:15 | access to array indirection | tests.cpp:622:19:622:24 | source indirection |
715
nodes
816
| main.cpp:6:27:6:30 | argv indirection | semmle.label | argv indirection |
917
| main.cpp:10:20:10:23 | argv indirection | semmle.label | argv indirection |
1018
| tests.cpp:613:19:613:24 | source indirection | semmle.label | source indirection |
1119
| tests.cpp:615:17:615:22 | source indirection | semmle.label | source indirection |
20+
| tests.cpp:622:19:622:24 | source indirection | semmle.label | source indirection |
21+
| tests.cpp:625:2:625:16 | ... = ... indirection | semmle.label | ... = ... indirection |
22+
| tests.cpp:625:4:625:7 | s indirection [post update] [home indirection] | semmle.label | s indirection [post update] [home indirection] |
23+
| tests.cpp:628:14:628:14 | s indirection [home indirection] | semmle.label | s indirection [home indirection] |
24+
| tests.cpp:628:14:628:19 | home indirection | semmle.label | home indirection |
25+
| tests.cpp:628:16:628:19 | home indirection | semmle.label | home indirection |
1226
| tests.cpp:631:32:631:35 | argv indirection | semmle.label | argv indirection |
1327
| tests.cpp:656:9:656:15 | access to array indirection | semmle.label | access to array indirection |
28+
| tests.cpp:657:9:657:15 | access to array indirection | semmle.label | access to array indirection |
1429
subpaths
1530
#select
1631
| tests.cpp:615:2:615:7 | call to strcpy | main.cpp:6:27:6:30 | argv indirection | tests.cpp:615:17:615:22 | source indirection | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | argv indirection | a command-line argument |
32+
| tests.cpp:628:2:628:7 | call to strcpy | main.cpp:6:27:6:30 | argv indirection | tests.cpp:628:14:628:19 | home indirection | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | argv indirection | a command-line argument |

0 commit comments

Comments
 (0)