Skip to content

Commit 58f6058

Browse files
MathiasVPMathiasVP
authored
Merge pull request #7051 from MathiasVP/better-paths-in-tests
C++: Better `InlineExpectation` tests for path-explanations
2 parents 061fc16 + 0d1ff4d commit 58f6058

File tree

4 files changed

+80
-58
lines changed

4 files changed

+80
-58
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,24 @@ module TaintedWithPath {
474474
}
475475
}
476476

477+
/**
478+
* INTERNAL: Do not use.
479+
*/
480+
module Private {
481+
/** Gets a predecessor `PathNode` of `pathNode`, if any. */
482+
PathNode getAPredecessor(PathNode pathNode) { edges(result, pathNode) }
483+
484+
/** Gets the element that `pathNode` wraps, if any. */
485+
Element getElementFromPathNode(PathNode pathNode) {
486+
exists(DataFlow::Node node | node = pathNode.(WrapPathNode).inner().getNode() |
487+
result = node.asExpr() or
488+
result = node.asParameter()
489+
)
490+
or
491+
result = pathNode.(EndpointPathNode).inner()
492+
}
493+
}
494+
477495
private class WrapPathNode extends PathNode, TWrapPathNode {
478496
DataFlow3::PathNode inner() { this = TWrapPathNode(result) }
479497

‎cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/dispatch.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ using SinkFunction = void (*)(int);
44

55
void notSink(int notSinkParam);
66

7-
void callsSink(int sinkParam) {
8-
sink(sinkParam); // $ ast,ir=31:28 ast,ir=32:31 ast,ir=34:22 MISSING: ast,ir=28
7+
void callsSink(int sinkParam) { // $ ir-path=31:28 ir-path=32:31 ir-path=34:22
8+
sink(sinkParam); // $ ir-sink=31:28 ir-sink=32:31 ir-sink=34:22 ast=31:28 ast=32:31 ast=34:22 MISSING: ast,ir=28
99
}
1010

1111
struct {
@@ -25,13 +25,13 @@ void assignGlobals() {
2525
};
2626

2727
void testStruct() {
28-
globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // $ MISSING: ast,ir
28+
globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // $ ir MISSING: ast
2929
globalStruct.notSinkPtr(atoi(getenv("TAINTED"))); // clean
3030

31-
globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // $ ast,ir
32-
globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // $ ast,ir
31+
globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
32+
globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
3333

34-
globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast,ir
34+
globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
3535
}
3636

3737
class B {
@@ -48,19 +48,19 @@ class D2 : public D1 {
4848

4949
class D3 : public D2 {
5050
public:
51-
void f(const char* p) override {
52-
sink(p); // $ ast,ir=58:10 ast,ir=60:17 ast,ir=61:28 ast,ir=62:29 ast,ir=63:33 SPURIOUS: ast,ir=73:30
51+
void f(const char* p) override { // $ ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
52+
sink(p); // $ ir-sink=58:10 ir-sink=60:17 ir-sink=61:28 ir-sink=62:29 ir-sink=63:33 ast=58:10 ast=60:17 ast=61:28 ast=62:29 ast=63:33 SPURIOUS: ast=73:30 ir-sink=73:30
5353
}
5454
};
5555

5656
void test_dynamic_cast() {
5757
B* b = new D3();
58-
b->f(getenv("VAR")); // $ ast,ir
58+
b->f(getenv("VAR")); // $ ast ir-path
5959

60-
((D2*)b)->f(getenv("VAR")); // $ ast,ir
61-
static_cast<D2*>(b)->f(getenv("VAR")); // $ ast,ir
62-
dynamic_cast<D2*>(b)->f(getenv("VAR")); // $ ast,ir
63-
reinterpret_cast<D2*>(b)->f(getenv("VAR")); // $ ast,ir
60+
((D2*)b)->f(getenv("VAR")); // $ ast ir-path
61+
static_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
62+
dynamic_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
63+
reinterpret_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
6464

6565
B* b2 = new D2();
6666
b2->f(getenv("VAR"));
@@ -70,5 +70,5 @@ void test_dynamic_cast() {
7070
dynamic_cast<D2*>(b2)->f(getenv("VAR"));
7171
reinterpret_cast<D2*>(b2)->f(getenv("VAR"));
7272

73-
dynamic_cast<D3*>(b2)->f(getenv("VAR")); // $ SPURIOUS: ast,ir
73+
dynamic_cast<D3*>(b2)->f(getenv("VAR")); // $ SPURIOUS: ast ir-path
7474
}

‎cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/tainted.ql

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import cpp
77
import semmle.code.cpp.security.TaintTrackingImpl as ASTTaintTracking
88
import semmle.code.cpp.ir.dataflow.DefaultTaintTracking as IRDefaultTaintTracking
99
import IRDefaultTaintTracking::TaintedWithPath as TaintedWithPath
10+
import TaintedWithPath::Private
1011
import TestUtilities.InlineExpectationsTest
1112

12-
predicate isSink(Element sink) {
13+
predicate isSinkArgument(Element sink) {
1314
exists(FunctionCall call |
1415
call.getTarget().getName() = "sink" and
1516
sink = call.getAnArgument()
@@ -19,31 +20,34 @@ predicate isSink(Element sink) {
1920
predicate astTaint(Expr source, Element sink) { ASTTaintTracking::tainted(source, sink) }
2021

2122
class SourceConfiguration extends TaintedWithPath::TaintTrackingConfiguration {
22-
override predicate isSink(Element e) { any() }
23+
override predicate isSink(Element e) { isSinkArgument(e) }
2324
}
2425

25-
predicate irTaint(Expr source, Element sink) {
26-
TaintedWithPath::taintedWithPath(source, sink, _, _)
26+
predicate irTaint(Element source, Element sink, string tag) {
27+
exists(TaintedWithPath::PathNode sinkNode, TaintedWithPath::PathNode predNode |
28+
TaintedWithPath::taintedWithPath(source, _, _, sinkNode) and
29+
predNode = getAPredecessor*(sinkNode) and
30+
sink = getElementFromPathNode(predNode) and
31+
// Make sure the path is actually reachable from this predecessor.
32+
// Otherwise, we could pick `predNode` to be b when `source` is
33+
// `source1` in this dataflow graph:
34+
// source1 ---> a ---> c ---> sinkNode
35+
// ^
36+
// source2 ---> b --/
37+
source = getElementFromPathNode(getAPredecessor*(predNode)) and
38+
if sinkNode = predNode then tag = "ir-sink" else tag = "ir-path"
39+
)
2740
}
2841

2942
class IRDefaultTaintTrackingTest extends InlineExpectationsTest {
3043
IRDefaultTaintTrackingTest() { this = "IRDefaultTaintTrackingTest" }
3144

32-
override string getARelevantTag() { result = "ir" }
45+
override string getARelevantTag() { result = ["ir-path", "ir-sink"] }
3346

3447
override predicate hasActualResult(Location location, string element, string tag, string value) {
35-
exists(Expr source, Element tainted, int n |
36-
tag = "ir" and
37-
irTaint(source, tainted) and
38-
(
39-
isSink(tainted)
40-
or
41-
exists(Element sink |
42-
isSink(sink) and
43-
irTaint(tainted, sink)
44-
)
45-
) and
46-
n = strictcount(Expr otherSource | irTaint(otherSource, tainted)) and
48+
exists(Element source, Element tainted, int n |
49+
irTaint(source, tainted, tag) and
50+
n = strictcount(Element otherSource | irTaint(otherSource, tainted, _)) and
4751
(
4852
n = 1 and value = ""
4953
or
@@ -70,10 +74,10 @@ class ASTTaintTrackingTest extends InlineExpectationsTest {
7074
tag = "ast" and
7175
astTaint(source, tainted) and
7276
(
73-
isSink(tainted)
77+
isSinkArgument(tainted)
7478
or
7579
exists(Element sink |
76-
isSink(sink) and
80+
isSinkArgument(sink) and
7781
astTaint(tainted, sink)
7882
)
7983
) and

‎cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/test_diff.cpp

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ struct S {
1313
}
1414
};
1515

16-
void calls_sink_with_argv(const char* a) {
17-
sink(a); // $ ast,ir=96:26 ast,ir=98:18
16+
void calls_sink_with_argv(const char* a) { // $ ir-path=96:26 ir-path=98:18
17+
sink(a); // $ ast=96:26 ast=98:18 ir-sink=96:26 ir-sink=98:18
1818
}
1919

2020
extern int i;
@@ -26,8 +26,8 @@ class BaseWithPureVirtual {
2626

2727
class DerivedCallsSink : public BaseWithPureVirtual {
2828
public:
29-
void f(const char* p) override {
30-
sink(p); // $ ir ast=108:10 SPURIOUS: ast=111:10
29+
void f(const char* p) override { // $ ir-path
30+
sink(p); // $ ir-sink ast=108:10 SPURIOUS: ast=111:10
3131
}
3232
};
3333

@@ -38,8 +38,8 @@ class DerivedDoesNotCallSink : public BaseWithPureVirtual {
3838

3939
class DerivedCallsSinkDiamond1 : virtual public BaseWithPureVirtual {
4040
public:
41-
void f(const char* p) override {
42-
sink(p); // $ ast,ir
41+
void f(const char* p) override { // $ ir-path
42+
sink(p); // $ ast ir-sink
4343
}
4444
};
4545

@@ -49,23 +49,23 @@ class DerivedDoesNotCallSinkDiamond2 : virtual public BaseWithPureVirtual {
4949
};
5050

5151
class DerivesMultiple : public DerivedCallsSinkDiamond1, public DerivedDoesNotCallSinkDiamond2 {
52-
void f(const char* p) override {
52+
void f(const char* p) override { // $ ir-path
5353
DerivedCallsSinkDiamond1::f(p);
5454
}
5555
};
5656

5757
template<typename T>
5858
class CRTP {
5959
public:
60-
void f(const char* p) {
60+
void f(const char* p) { // $ ir-path
6161
static_cast<T*>(this)->g(p);
6262
}
6363
};
6464

6565
class CRTPCallsSink : public CRTP<CRTPCallsSink> {
6666
public:
67-
void g(const char* p) {
68-
sink(p); // $ ast,ir
67+
void g(const char* p) { // $ ir-path
68+
sink(p); // $ ast ir-sink
6969
}
7070
};
7171

@@ -78,8 +78,8 @@ class Derived2 : public Derived1 {
7878

7979
class Derived3 : public Derived2 {
8080
public:
81-
void f(const char* p) override {
82-
sink(p); // $ ast,ir=124:19 ast,ir=126:43 ast,ir=128:44
81+
void f(const char* p) override { // $ ir-path=124:19 ir-path=126:43 ir-path=128:44
82+
sink(p); // $ ast,ir-sink=124:19 ast,ir-sink=126:43 ast,ir-sink=128:44
8383
}
8484
};
8585

@@ -89,41 +89,41 @@ class CRTPDoesNotCallSink : public CRTP<CRTPDoesNotCallSink> {
8989
};
9090

9191
int main(int argc, char *argv[]) {
92-
sink(argv[0]); // $ ast,ir
92+
sink(argv[0]); // $ ast,ir-path,ir-sink
9393

94-
sink(reinterpret_cast<int>(argv)); // $ ast,ir
94+
sink(reinterpret_cast<int>(argv)); // $ ast,ir-sink
9595

96-
calls_sink_with_argv(argv[1]); // $ ast,ir
96+
calls_sink_with_argv(argv[1]); // $ ast,ir-path
9797

98-
char*** p = &argv; // $ ast,ir
98+
char*** p = &argv; // $ ast,ir-path
9999

100-
sink(*p[0]); // $ ast,ir
100+
sink(*p[0]); // $ ast,ir-sink
101101

102-
calls_sink_with_argv(*p[i]); // $ MISSING: ast,ir
102+
calls_sink_with_argv(*p[i]); // $ MISSING: ast,ir-path
103103

104-
sink(*(argv + 1)); // $ ast,ir
104+
sink(*(argv + 1)); // $ ast,ir-path ir-sink
105105

106106
BaseWithPureVirtual* b = new DerivedCallsSink;
107107

108-
b->f(argv[1]); // $ ast,ir
108+
b->f(argv[1]); // $ ast,ir-path
109109

110110
b = new DerivedDoesNotCallSink;
111111
b->f(argv[0]); // $ SPURIOUS: ast
112112

113113
BaseWithPureVirtual* b2 = new DerivesMultiple;
114114

115-
b2->f(argv[i]); // $ ast,ir
115+
b2->f(argv[i]); // $ ast,ir-path
116116

117117
CRTP<CRTPDoesNotCallSink> crtp_not_call_sink;
118118
crtp_not_call_sink.f(argv[0]); // clean
119119

120120
CRTP<CRTPCallsSink> crtp_calls_sink;
121-
crtp_calls_sink.f(argv[0]); // $ ast,ir
121+
crtp_calls_sink.f(argv[0]); // $ ast,ir-path
122122

123123
Derived1* calls_sink = new Derived3;
124-
calls_sink->f(argv[1]); // $ ast,ir
124+
calls_sink->f(argv[1]); // $ ast,ir-path
125125

126-
static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir
126+
static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir-path
127127

128-
dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir
128+
dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir-path
129129
}

0 commit comments

Comments
 (0)