hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-27 17:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2713 from MathiasVP/dynamic-cast-taint-propagation

Browse Source 
C++: Taint propagation through dynamic_cast
This commit is contained in:
Robert Marsh
2020-01-28 15:09:49 -05:00
committed by GitHub
parent 97069a7988 67d29e31cc
commit 9504da54d1
11 changed files with 131 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -268,6 +268,7 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
iTo.(PhiInstruction).getAnOperand().getDef() = iFrom or
// Treat all conversions as flow, even conversions between different numeric types.
iTo.(ConvertInstruction).getUnary() = iFrom or
iTo.(CheckedConvertOrNullInstruction).getUnary() = iFrom or
iTo.(InheritanceConversionInstruction).getUnary() = iFrom or
// A chi instruction represents a point where a new value (the _partial_
// operand) may overwrite an old value (the _total_ operand), but the alias

4
cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll
View File

@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
}
class CheckedConvertOrNullInstruction extends UnaryInstruction {
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
}
/**
* Represents an instruction that converts between two addresses
* related by inheritance.

4
cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll
View File

@@ -96,6 +96,10 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) {
bitOffset = Ints::mul(convert.getDerivation().getByteOffset(), 8)
)
or
// Conversion using dynamic_cast results in an unknown offset
instr instanceof CheckedConvertOrNullInstruction and
bitOffset = Ints::unknown()
or
// Converting to a derived class subtracts the offset of the base class.
exists(ConvertToDerivedInstruction convert |
convert = instr and

4
cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll
View File

@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
}
class CheckedConvertOrNullInstruction extends UnaryInstruction {
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
}
/**
* Represents an instruction that converts between two addresses
* related by inheritance.

4
cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll
View File

@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
}
class CheckedConvertOrNullInstruction extends UnaryInstruction {
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
}
/**
* Represents an instruction that converts between two addresses
* related by inheritance.

4
cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll
View File

@@ -96,6 +96,10 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) {
bitOffset = Ints::mul(convert.getDerivation().getByteOffset(), 8)
)
or
// Conversion using dynamic_cast results in an unknown offset
instr instanceof CheckedConvertOrNullInstruction and
bitOffset = Ints::unknown()
or
// Converting to a derived class subtracts the offset of the base class.
exists(ConvertToDerivedInstruction convert |
convert = instr and

39
cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp
View File

@@ -39,3 +39,42 @@ void test_indirect_arg_to_model() {
inet_addr_retval a = inet_addr((const char *)&env_pointer);
sink(a);
}
class B {
public:
virtual void f(const char*) = 0;
};
class D1 : public B {};
class D2 : public D1 {
public:
void f(const char* p) override {}
};
class D3 : public D2 {
public:
void f(const char* p) override {
sink(p);
}
};
void test_dynamic_cast() {
B* b = new D3();
b->f(getenv("VAR")); // tainted
((D2*)b)->f(getenv("VAR")); // tainted
static_cast<D2*>(b)->f(getenv("VAR")); // tainted
dynamic_cast<D2*>(b)->f(getenv("VAR")); // tainted
reinterpret_cast<D2*>(b)->f(getenv("VAR")); // tainted
B* b2 = new D2();
b2->f(getenv("VAR"));
((D2*)b2)->f(getenv("VAR"));
static_cast<D2*>(b2)->f(getenv("VAR"));
dynamic_cast<D2*>(b2)->f(getenv("VAR"));
reinterpret_cast<D2*>(b2)->f(getenv("VAR"));
dynamic_cast<D3*>(b2)->f(getenv("VAR")); // tainted [FALSE POSITIVE]
}

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

@@ -30,6 +30,64 @@
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:26:39:34 | call to inet_addr |
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:50:39:61 | & ... |
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:40:10:40:10 | a |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:45:20:45:29 | p#0 |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:64:10:64:15 | call to getenv |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:64:10:64:22 | (const char *)... |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:66:17:66:22 | call to getenv |
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:66:17:66:29 | (const char *)... |
| defaulttainttracking.cpp:66:17:66:22 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:67:28:67:33 | call to getenv |
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:67:28:67:40 | (const char *)... |
| defaulttainttracking.cpp:67:28:67:33 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:68:29:68:34 | call to getenv |
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:68:29:68:41 | (const char *)... |
| defaulttainttracking.cpp:68:29:68:34 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:69:33:69:38 | call to getenv |
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:69:33:69:45 | (const char *)... |
| defaulttainttracking.cpp:69:33:69:38 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:45:20:45:29 | p#0 |
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:72:11:72:16 | call to getenv |
| defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:72:11:72:23 | (const char *)... |
| defaulttainttracking.cpp:74:18:74:23 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:74:18:74:23 | call to getenv | defaulttainttracking.cpp:74:18:74:23 | call to getenv |
| defaulttainttracking.cpp:74:18:74:23 | call to getenv | defaulttainttracking.cpp:74:18:74:30 | (const char *)... |
| defaulttainttracking.cpp:75:29:75:34 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:75:29:75:34 | call to getenv | defaulttainttracking.cpp:75:29:75:34 | call to getenv |
| defaulttainttracking.cpp:75:29:75:34 | call to getenv | defaulttainttracking.cpp:75:29:75:41 | (const char *)... |
| defaulttainttracking.cpp:76:30:76:35 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:76:30:76:35 | call to getenv | defaulttainttracking.cpp:76:30:76:35 | call to getenv |
| defaulttainttracking.cpp:76:30:76:35 | call to getenv | defaulttainttracking.cpp:76:30:76:42 | (const char *)... |
| defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p |
| defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:77:34:77:39 | call to getenv |
| defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:77:34:77:46 | (const char *)... |
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p |
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p |
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:79:30:79:35 | call to getenv |
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:79:30:79:42 | (const char *)... |
| defaulttainttracking.cpp:79:30:79:35 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
| test_diff.cpp:92:10:92:13 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:1:11:1:20 | p#0 |
| test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:92:10:92:13 | argv |
@@ -123,7 +181,11 @@
| test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:46 | argv |
| test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:49 | (const char *)... |
| test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:49 | access to array |
| test_diff.cpp:128:44:128:47 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:1:11:1:20 | p#0 |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:76:24:76:24 | p |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:81:24:81:24 | p |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:82:14:82:14 | p |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:128:44:128:47 | argv |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:128:44:128:50 | (const char *)... |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:128:44:128:50 | access to array |

5
cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected
View File

@@ -9,6 +9,7 @@
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:31:40:31:53 | dotted_address | AST only |
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:36:39:61 | (const char *)... | AST only |
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:51:39:61 | env_pointer | AST only |
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | IR only |
| test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only |
| test_diff.cpp:108:10:108:13 | argv | test_diff.cpp:36:24:36:24 | p | AST only |
| test_diff.cpp:111:10:111:13 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | AST only |
@@ -16,7 +17,3 @@
| test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:29:24:29:24 | p | AST only |
| test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:30:14:30:14 | p | AST only |
| test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:76:24:76:24 | p | IR only |
| test_diff.cpp:128:44:128:47 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | AST only |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:1:11:1:20 | p#0 | AST only |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:81:24:81:24 | p | AST only |
| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:82:14:82:14 | p | AST only |

4
csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll
View File

@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
}
class CheckedConvertOrNullInstruction extends UnaryInstruction {
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
}
/**
* Represents an instruction that converts between two addresses
* related by inheritance.

4
csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll
View File

@@ -947,6 +947,10 @@ class ConvertInstruction extends UnaryInstruction {
ConvertInstruction() { getOpcode() instanceof Opcode::Convert }
}
class CheckedConvertOrNullInstruction extends UnaryInstruction {
CheckedConvertOrNullInstruction() { getOpcode() instanceof Opcode::CheckedConvertOrNull }
}
/**
* Represents an instruction that converts between two addresses
* related by inheritance.