Skip to content

Commit ae4b6c5

Browse files
MathiasVPMathiasVP
committed
C++: Change the structure of the 'annotate_path_to_sink' tests to better test path-explanations.
1 parent ab37ae6 commit ae4b6c5

File tree

4 files changed

+72
-58
lines changed

4 files changed

+72
-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=31:28 ir-path=32:31 ir-path=34:22
32+
globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path=31:28 ir-path=32:31 ir-path=34:22
3333

34-
globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast,ir
34+
globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path=31:28 ir-path=32:31 ir-path=34:22
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=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
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=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
61+
static_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
62+
dynamic_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
63+
reinterpret_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
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=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
7474
}

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

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import semmle.code.cpp.ir.dataflow.DefaultTaintTracking as IRDefaultTaintTrackin
99
import IRDefaultTaintTracking::TaintedWithPath as TaintedWithPath
1010
import TestUtilities.InlineExpectationsTest
1111

12-
predicate isSink(Element sink) {
12+
predicate isSinkArgument(Element sink) {
1313
exists(FunctionCall call |
1414
call.getTarget().getName() = "sink" and
1515
sink = call.getAnArgument()
@@ -19,31 +19,27 @@ predicate isSink(Element sink) {
1919
predicate astTaint(Expr source, Element sink) { ASTTaintTracking::tainted(source, sink) }
2020

2121
class SourceConfiguration extends TaintedWithPath::TaintTrackingConfiguration {
22-
override predicate isSink(Element e) { any() }
22+
override predicate isSink(Element e) { isSinkArgument(e) }
2323
}
2424

25-
predicate irTaint(Expr source, Element sink) {
26-
TaintedWithPath::taintedWithPath(source, sink, _, _)
25+
predicate irTaint(Element source, Element sink, string tag) {
26+
exists(TaintedWithPath::PathNode sinkNode, TaintedWithPath::PathNode predNode |
27+
TaintedWithPath::taintedWithPath(source, _, _, sinkNode) and
28+
predNode = TaintedWithPath::Private::getAPredecessor*(sinkNode) and
29+
sink = TaintedWithPath::Private::getElementFromPathNode(predNode) and
30+
if sinkNode = predNode then tag = "ir-sink" else tag = "ir-path"
31+
)
2732
}
2833

2934
class IRDefaultTaintTrackingTest extends InlineExpectationsTest {
3035
IRDefaultTaintTrackingTest() { this = "IRDefaultTaintTrackingTest" }
3136

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

3439
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
40+
exists(Element source, Element tainted, int n |
41+
irTaint(source, tainted, tag) and
42+
n = strictcount(Element otherSource | irTaint(otherSource, tainted, _)) and
4743
(
4844
n = 1 and value = ""
4945
or
@@ -70,10 +66,10 @@ class ASTTaintTrackingTest extends InlineExpectationsTest {
7066
tag = "ast" and
7167
astTaint(source, tainted) and
7268
(
73-
isSink(tainted)
69+
isSinkArgument(tainted)
7470
or
7571
exists(Element sink |
76-
isSink(sink) and
72+
isSinkArgument(sink) and
7773
astTaint(tainted, sink)
7874
)
7975
) 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); // $ ir-sink=124:19 ir-sink=126:43 ir-sink=128:44 ast,ir=124:19 ast,ir=126:43 ast,ir=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=96:26 ir-path=98:18
9797

98-
char*** p = &argv; // $ ast,ir
98+
char*** p = &argv; // $ ast ir-path=96:26 ir-path=98:18
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=124:19 ir-path=126:43 ir-path=128:44
125125

126-
static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir
126+
static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast ir-path=124:19 ir-path=126:43 ir-path=128:44
127127

128-
dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir
128+
dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast ir-path=124:19 ir-path=126:43 ir-path=128:44
129129
}

0 commit comments

Comments
 (0)