Skip to content

Commit af8bbf4

Browse files
committed
C++: Some data flow through partial chi operands
1 parent adc557f commit af8bbf4

File tree

5 files changed

+21
-8
lines changed

5 files changed

+21
-8
lines changed

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,18 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
278278
// By allowing flow through the total operand, we ensure that flow is not lost
279279
// due to shortcomings of the alias analysis. We may get false flow in cases
280280
// where the data is indeed overwritten.
281-
//
282-
// Allowing flow through the partial operand would be more noisy, especially
283-
// for variables that have escaped: for soundness, the IR has to assume that
284-
// every write to an unknown address can affect every escaped variable, and
285-
// this assumption shows up as data flowing through partial chi operands.
286281
iTo.getAnOperand().(ChiTotalOperand).getDef() = iFrom
282+
or
283+
// Flow through the partial operand must be restricted a bit more. For
284+
// soundness, the IR has to assume that every write to an unknown address can
285+
// affect every escaped variable, and this assumption shows up as data flowing
286+
// through partial chi operands. The chi instructions for all escaped data can
287+
// be recognized by having unknown types. For all other chi instructions, flow
288+
// through partial operands is more likely to be real.
289+
exists(ChiInstruction chi | iTo = chi |
290+
iFrom = chi.getPartial() and
291+
not chi.getResultIRType() instanceof IRUnknownType
292+
)
287293
}
288294

289295
/**

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:33 | (const char *)... |
2626
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:25 | call to getenv |
2727
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:32 | (const char *)... |
28+
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | buf |
2829
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:32:11:32:26 | p#0 |
2930
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:11:38:21 | env_pointer |
3031
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:25:38:30 | call to getenv |

cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,9 @@ void array_test(int i) {
107107
arr3[5] = 0;
108108

109109
sink(arr1[5]); // tainted
110-
sink(arr1[i]); // tainted [NOT DETECTED]
111-
sink(arr2[5]); // tainted [NOT DETECTED]
112-
sink(arr2[i]); // tainted [NOT DETECTED]
110+
sink(arr1[i]); // tainted [NOT DETECTED with AST]
111+
sink(arr2[5]); // tainted [NOT DETECTED with AST]
112+
sink(arr2[i]); // tainted [NOT DETECTED with AST]
113113
sink(arr3[5]);
114114
sink(arr3[i]);
115115
}

cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
| taint.cpp:93:11:93:11 | taint.cpp:71:22:71:27 | AST only |
88
| taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
99
| taint.cpp:109:7:109:13 | taint.cpp:105:12:105:17 | IR only |
10+
| taint.cpp:110:7:110:13 | taint.cpp:105:12:105:17 | IR only |
11+
| taint.cpp:111:7:111:13 | taint.cpp:106:12:106:17 | IR only |
12+
| taint.cpp:112:7:112:13 | taint.cpp:106:12:106:17 | IR only |
1013
| taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
1114
| taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
1215
| taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |

cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
| taint.cpp:16:8:16:14 | source1 | taint.cpp:12:22:12:27 | call to source |
33
| taint.cpp:17:8:17:16 | ++ ... | taint.cpp:12:22:12:27 | call to source |
44
| taint.cpp:109:7:109:13 | access to array | taint.cpp:105:12:105:17 | call to source |
5+
| taint.cpp:110:7:110:13 | access to array | taint.cpp:105:12:105:17 | call to source |
6+
| taint.cpp:111:7:111:13 | access to array | taint.cpp:106:12:106:17 | call to source |
7+
| taint.cpp:112:7:112:13 | access to array | taint.cpp:106:12:106:17 | call to source |
58
| taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
69
| taint.cpp:130:7:130:9 | * ... | taint.cpp:127:8:127:13 | call to source |
710
| taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |

0 commit comments

Comments
 (0)