Skip to content

Commit 9504da5

Browse files
rdmarsh2rdmarsh2
authored
Merge pull request #2713 from MathiasVP/dynamic-cast-taint-propagation
C++: Taint propagation through dynamic_cast
2 parents 97069a7 + 67d29e3 commit 9504da5

File tree

11 files changed

+131
-4
lines changed

11 files changed

+131
-4
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
268268
iTo.(PhiInstruction).getAnOperand().getDef() = iFrom or
269269
// Treat all conversions as flow, even conversions between different numeric types.
270270
iTo.(ConvertInstruction).getUnary() = iFrom or
271+
iTo.(CheckedConvertOrNullInstruction).getUnary() = iFrom or
271272
iTo.(InheritanceConversionInstruction).getUnary() = iFrom or
272273
// A chi instruction represents a point where a new value (the _partial_
273274
// operand) may overwrite an old value (the _total_ operand), but the alias

‎cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
947947
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
948948
}
949949

950+
class CheckedConvertOrNullInstruction extends UnaryInstruction {
951+
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
952+
}
953+
950954
/**
951955
* Represents an instruction that converts between two addresses
952956
* related by inheritance.

‎cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) {
9696
bitOffset = Ints::mul(convert.getDerivation().getByteOffset(), 8)
9797
)
9898
or
99+
// Conversion using dynamic_cast results in an unknown offset
100+
instr instanceof CheckedConvertOrNullInstruction and
101+
bitOffset = Ints::unknown()
102+
or
99103
// Converting to a derived class subtracts the offset of the base class.
100104
exists(ConvertToDerivedInstruction convert |
101105
convert = instr and

‎cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
947947
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
948948
}
949949

950+
class CheckedConvertOrNullInstruction extends UnaryInstruction {
951+
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
952+
}
953+
950954
/**
951955
* Represents an instruction that converts between two addresses
952956
* related by inheritance.

‎cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
947947
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
948948
}
949949

950+
class CheckedConvertOrNullInstruction extends UnaryInstruction {
951+
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
952+
}
953+
950954
/**
951955
* Represents an instruction that converts between two addresses
952956
* related by inheritance.

‎cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) {
9696
bitOffset = Ints::mul(convert.getDerivation().getByteOffset(), 8)
9797
)
9898
or
99+
// Conversion using dynamic_cast results in an unknown offset
100+
instr instanceof CheckedConvertOrNullInstruction and
101+
bitOffset = Ints::unknown()
102+
or
99103
// Converting to a derived class subtracts the offset of the base class.
100104
exists(ConvertToDerivedInstruction convert |
101105
convert = instr and

‎cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,42 @@ void test_indirect_arg_to_model() {
3939
inet_addr_retval a = inet_addr((const char *)&env_pointer);
4040
sink(a);
4141
}
42+
43+
class B {
44+
public:
45+
virtual void f(const char*) = 0;
46+
};
47+
48+
class D1 : public B {};
49+
50+
class D2 : public D1 {
51+
public:
52+
void f(const char* p) override {}
53+
};
54+
55+
class D3 : public D2 {
56+
public:
57+
void f(const char* p) override {
58+
sink(p);
59+
}
60+
};
61+
62+
void test_dynamic_cast() {
63+
B* b = new D3();
64+
b->f(getenv("VAR")); // tainted
65+
66+
((D2*)b)->f(getenv("VAR")); // tainted
67+
static_cast<D2*>(b)->f(getenv("VAR")); // tainted
68+
dynamic_cast<D2*>(b)->f(getenv("VAR")); // tainted
69+
reinterpret_cast<D2*>(b)->f(getenv("VAR")); // tainted
70+
71+
B* b2 = new D2();
72+
b2->f(getenv("VAR"));
73+
74+
((D2*)b2)->f(getenv("VAR"));
75+
static_cast<D2*>(b2)->f(getenv("VAR"));
76+
dynamic_cast<D2*>(b2)->f(getenv("VAR"));
77+
reinterpret_cast<D2*>(b2)->f(getenv("VAR"));
78+
79+
dynamic_cast<D3*>(b2)->f(getenv("VAR")); // tainted [FALSE POSITIVE]
80+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,64 @@
3030
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:26:39:34 | call to inet_addr |
3131
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:50:39:61 | & ... |
3232
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:40:10:40:10 | a |
33+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
34+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:45:20:45:29 | p#0 |
35+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
36+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
37+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
38+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:64:10:64:15 | call to getenv |
39+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:64:10:64:22 | (const char *)... |
40+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
41+
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
42+
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
43+
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
44+
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
45+
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:66:17:66:22 | call to getenv |
46+
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:66:17:66:29 | (const char *)... |
47+
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
48+
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
49+
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
50+
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
51+
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
52+
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:67:28:67:33 | call to getenv |
53+
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:67:28:67:40 | (const char *)... |
54+
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
55+
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
56+
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
57+
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
58+
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
59+
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:68:29:68:34 | call to getenv |
60+
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:68:29:68:41 | (const char *)... |
61+
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
62+
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
63+
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
64+
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
65+
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
66+
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:69:33:69:38 | call to getenv |
67+
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:69:33:69:45 | (const char *)... |
68+
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
69+
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:45:20:45:29 | p#0 |
70+
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
71+
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:72:11:72:16 | call to getenv |
72+
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:72:11:72:23 | (const char *)... |
73+
| defaulttainttracking.cpp:74:18:74:23 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
74+
| defaulttainttracking.cpp:74:18:74:23 | call to getenv | defaulttainttracking.cpp:74:18:74:23 | call to getenv |
75+
| defaulttainttracking.cpp:74:18:74:23 | call to getenv | defaulttainttracking.cpp:74:18:74:30 | (const char *)... |
76+
| defaulttainttracking.cpp:75:29:75:34 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
77+
| defaulttainttracking.cpp:75:29:75:34 | call to getenv | defaulttainttracking.cpp:75:29:75:34 | call to getenv |
78+
| defaulttainttracking.cpp:75:29:75:34 | call to getenv | defaulttainttracking.cpp:75:29:75:41 | (const char *)... |
79+
| defaulttainttracking.cpp:76:30:76:35 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
80+
| defaulttainttracking.cpp:76:30:76:35 | call to getenv | defaulttainttracking.cpp:76:30:76:35 | call to getenv |
81+
| defaulttainttracking.cpp:76:30:76:35 | call to getenv | defaulttainttracking.cpp:76:30:76:42 | (const char *)... |
82+
| defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
83+
| defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:77:34:77:39 | call to getenv |
84+
| defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:77:34:77:46 | (const char *)... |
85+
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
86+
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
87+
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
88+
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:79:30:79:35 | call to getenv |
89+
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:79:30:79:42 | (const char *)... |
90+
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
3391
| test_diff.cpp:92:10:92:13 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
3492
| test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:1:11:1:20 | p#0 |
3593
| test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:92:10:92:13 | argv |
@@ -123,7 +181,11 @@
123181
| test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:46 | argv |
124182
| test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:49 | (const char *)... |
125183
| test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:49 | access to array |
184+
| test_diff.cpp:128:44:128:47 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
185+
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:1:11:1:20 | p#0 |
126186
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:76:24:76:24 | p |
187+
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:81:24:81:24 | p |
188+
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:82:14:82:14 | p |
127189
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:128:44:128:47 | argv |
128190
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:128:44:128:50 | (const char *)... |
129191
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:128:44:128:50 | access to array |

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,11 @@
99
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:31:40:31:53 | dotted_address | AST only |
1010
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:36:39:61 | (const char *)... | AST only |
1111
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:51:39:61 | env_pointer | AST only |
12+
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | IR only |
1213
| test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only |
1314
| test_diff.cpp:108:10:108:13 | argv | test_diff.cpp:36:24:36:24 | p | AST only |
1415
| test_diff.cpp:111:10:111:13 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | AST only |
1516
| test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:1:11:1:20 | p#0 | AST only |
1617
| test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:29:24:29:24 | p | AST only |
1718
| test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:30:14:30:14 | p | AST only |
1819
| test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:76:24:76:24 | p | IR only |
19-
| test_diff.cpp:128:44:128:47 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | AST only |
20-
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:1:11:1:20 | p#0 | AST only |
21-
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:81:24:81:24 | p | AST only |
22-
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:82:14:82:14 | p | AST only |

‎csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
947947
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
948948
}
949949

950+
class CheckedConvertOrNullInstruction extends UnaryInstruction {
951+
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
952+
}
953+
950954
/**
951955
* Represents an instruction that converts between two addresses
952956
* related by inheritance.

0 commit comments

Comments
 (0)