diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 9548ff680744..7b1a9ca31238 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -645,6 +645,24 @@ class GlobalLikeVariable extends Variable { } } +/** + * Returns the smallest indirection for the type `t`. + * + * For most types this is `1`, but for `ArrayType`s (which are allocated on + * the stack) this is `0` + */ +int getMinIndirectionsForType(Type t) { + if t.getUnspecifiedType() instanceof Cpp::ArrayType then result = 0 else result = 1 +} + +private int getMinIndirectionForGlobalUse(Ssa::GlobalUse use) { + result = getMinIndirectionsForType(use.getUnspecifiedType()) +} + +private int getMinIndirectionForGlobalDef(Ssa::GlobalDef def) { + result = getMinIndirectionsForType(def.getUnspecifiedType()) +} + /** * Holds if data can flow from `node1` to `node2` in a way that loses the * calling context. For example, this would happen with flow through a @@ -656,7 +674,7 @@ predicate jumpStep(Node n1, Node n2) { v = globalUse.getVariable() and n1.(FinalGlobalValue).getGlobalUse() = globalUse | - globalUse.getIndirection() = 1 and + globalUse.getIndirection() = getMinIndirectionForGlobalUse(globalUse) and v = n2.asVariable() or v = n2.asIndirectVariable(globalUse.getIndirection()) @@ -666,7 +684,7 @@ predicate jumpStep(Node n1, Node n2) { v = globalDef.getVariable() and n2.(InitialGlobalValue).getGlobalDef() = globalDef | - globalDef.getIndirection() = 1 and + globalDef.getIndirection() = getMinIndirectionForGlobalDef(globalDef) and v = n1.asVariable() or v = n1.asIndirectVariable(globalDef.getIndirection()) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 17dae213cde1..07015db1c08a 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -34,7 +34,8 @@ cached private newtype TIRDataFlowNode = TNode0(Node0Impl node) { DataFlowImplCommon::forceCachingInSameStage() } or TVariableNode(Variable var, int indirectionIndex) { - indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())] + indirectionIndex = + [getMinIndirectionsForType(var.getUnspecifiedType()) .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())] } or TPostFieldUpdateNode(FieldAddress operand, int indirectionIndex) { indirectionIndex = @@ -346,7 +347,9 @@ class Node extends TIRDataFlowNode { * Gets the variable corresponding to this node, if any. This can be used for * modeling flow in and out of global variables. */ - Variable asVariable() { this = TVariableNode(result, 1) } + Variable asVariable() { + this = TVariableNode(result, getMinIndirectionsForType(result.getUnspecifiedType())) + } /** * Gets the `indirectionIndex`'th indirection of this node's underlying variable, if any. @@ -354,7 +357,7 @@ class Node extends TIRDataFlowNode { * This can be used for modeling flow in and out of global variables. */ Variable asIndirectVariable(int indirectionIndex) { - indirectionIndex > 1 and + indirectionIndex > getMinIndirectionsForType(result.getUnspecifiedType()) and this = TVariableNode(result, indirectionIndex) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll index fc718693dbdf..920172e429a5 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll @@ -872,7 +872,7 @@ private module Cached { upper = countIndirectionsForCppType(type) and ind = ind0 + [lower .. upper] and indirectionIndex = ind - (ind0 + lower) and - (if type.hasType(any(Cpp::ArrayType arrayType), true) then lower = 0 else lower = 1) + lower = getMinIndirectionsForType(any(Type t | type.hasUnspecifiedType(t, _))) ) } diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/TestBase.qll b/cpp/ql/test/library-tests/dataflow/dataflow-tests/TestBase.qll index 77087dbd3eec..528e7ca6ad33 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/TestBase.qll +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/TestBase.qll @@ -71,6 +71,14 @@ module IRTest { or source.asIndirectExpr(1).(FunctionCall).getTarget().getName() = "indirect_source" or + source.asExpr().(StringLiteral).getValue() = "source" + or + // indirect_source(n) gives the dataflow node representing the indirect node after n dereferences. + exists(int n, string s | + n = s.regexpCapture("indirect_source\\((\\d)\\)", 1).toInt() and + source.asIndirectExpr(n).(StringLiteral).getValue() = s + ) + or source.asParameter().getName().matches("source%") or source.(DataFlow::DefinitionByReferenceNode).getParameter().getName().matches("ref_source%") diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected index 068376778b2f..a98cfd7e22ac 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected @@ -1,5 +1,11 @@ uniqueEnclosingCallable +| test.cpp:864:44:864:58 | {...} | Node should have one enclosing callable but has 0. | +| test.cpp:864:47:864:54 | call to source | Node should have one enclosing callable but has 0. | +| test.cpp:872:46:872:51 | call to source | Node should have one enclosing callable but has 0. | +| test.cpp:872:53:872:56 | 1 | Node should have one enclosing callable but has 0. | uniqueCallEnclosingCallable +| test.cpp:864:47:864:54 | call to source | Call should have one enclosing callable but has 0. | +| test.cpp:872:46:872:51 | call to source | Call should have one enclosing callable but has 0. | uniqueType uniqueNodeLocation missingLocation diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected index 7e9f560ac0fb..c98bc68c884d 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected @@ -120,6 +120,7 @@ astFlow | test.cpp:797:22:797:28 | ref arg content | test.cpp:798:19:798:25 | content | | test.cpp:842:11:842:16 | call to source | test.cpp:844:8:844:8 | y | | test.cpp:846:13:846:27 | call to indirect_source | test.cpp:848:23:848:25 | rpx | +| test.cpp:860:54:860:59 | call to source | test.cpp:861:10:861:37 | static_local_pointer_dynamic | | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x | | true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x | | true_upon_entry.cpp:33:11:33:16 | call to source | true_upon_entry.cpp:39:8:39:8 | x | @@ -282,6 +283,17 @@ irFlow | test.cpp:832:21:832:26 | call to source | test.cpp:836:10:836:22 | global_direct | | test.cpp:842:11:842:16 | call to source | test.cpp:844:8:844:8 | y | | test.cpp:846:13:846:27 | call to indirect_source indirection | test.cpp:848:17:848:25 | rpx indirection | +| test.cpp:853:55:853:62 | call to source | test.cpp:854:10:854:36 | * ... | +| test.cpp:860:54:860:59 | call to source | test.cpp:861:10:861:37 | static_local_pointer_dynamic | +| test.cpp:872:46:872:51 | call to source | test.cpp:875:10:875:31 | global_pointer_dynamic | +| test.cpp:880:64:880:83 | indirect_source(1) indirection | test.cpp:883:10:883:45 | static_local_array_static_indirect_1 | +| test.cpp:881:64:881:83 | indirect_source(2) indirection | test.cpp:886:19:886:54 | static_local_array_static_indirect_2 indirection | +| test.cpp:890:54:890:61 | source | test.cpp:893:10:893:36 | static_local_pointer_static | +| test.cpp:891:65:891:84 | indirect_source(1) indirection | test.cpp:895:19:895:56 | static_local_pointer_static_indirect_1 indirection | +| test.cpp:901:56:901:75 | indirect_source(1) indirection | test.cpp:907:10:907:39 | global_array_static_indirect_1 | +| test.cpp:902:56:902:75 | indirect_source(2) indirection | test.cpp:911:19:911:48 | global_array_static_indirect_2 indirection | +| test.cpp:914:46:914:53 | source | test.cpp:919:10:919:30 | global_pointer_static | +| test.cpp:915:57:915:76 | indirect_source(1) indirection | test.cpp:921:19:921:50 | global_pointer_static_indirect_1 indirection | | true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x | | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x | | true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index 022ee63706a2..b5883963620d 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -1,5 +1,5 @@ int source(); -void sink(int); void sink(const int *); void sink(int **); void indirect_sink(...); +void sink(...); void indirect_sink(...); void intraprocedural_with_local_flow() { int t2; @@ -846,4 +846,80 @@ void test_references() { int* px = indirect_source(); int*& rpx = px; indirect_sink((int*)rpx); // $ ast,ir +} + +namespace GlobalArrays { + void test1() { + static const int static_local_array_dynamic[] = { ::source() }; + sink(*static_local_array_dynamic); // $ ir MISSING: ast + } + + const int* source(bool); + + void test2() { + static const int* static_local_pointer_dynamic = source(true); + sink(static_local_pointer_dynamic); // $ ast,ir + } + + static const int global_array_dynamic[] = { ::source() }; + + void test3() { + sink(*global_array_dynamic); // $ MISSING: ir,ast // Missing in IR because no 'IRFunction' for global_array is generated because the type of global_array_dynamic is "deeply const". + } + + const int* source(bool); + + static const int* global_pointer_dynamic = source(true); + + void test4() { + sink(global_pointer_dynamic); // $ ir MISSING: ast + } + + void test5() { + static const char static_local_array_static[] = "source"; + static const char static_local_array_static_indirect_1[] = "indirect_source(1)"; + static const char static_local_array_static_indirect_2[] = "indirect_source(2)"; + sink(static_local_array_static); // clean + sink(static_local_array_static_indirect_1); // $ ir MISSING: ast + indirect_sink(static_local_array_static_indirect_1); // clean + sink(static_local_array_static_indirect_2); // clean + indirect_sink(static_local_array_static_indirect_2); // $ ir MISSING: ast + } + + void test6() { + static const char* static_local_pointer_static = "source"; + static const char* static_local_pointer_static_indirect_1 = "indirect_source(1)"; + static const char* static_local_pointer_static_indirect_2 = "indirect_source(2)"; + sink(static_local_pointer_static); // $ ir MISSING: ast + sink(static_local_pointer_static_indirect_1); // clean + indirect_sink(static_local_pointer_static_indirect_1); // $ ir MISSING: ast + sink(static_local_pointer_static_indirect_2); // clean: static_local_pointer_static_indirect_2 does not have 2 indirections + indirect_sink(static_local_pointer_static_indirect_2); // clean: static_local_pointer_static_indirect_2 does not have 2 indirections + } + + static const char global_array_static[] = "source"; + static const char global_array_static_indirect_1[] = "indirect_source(1)"; + static const char global_array_static_indirect_2[] = "indirect_source(2)"; + + void test7() { + sink(global_array_static); // clean + sink(*global_array_static); // clean + sink(global_array_static_indirect_1); // $ ir MISSING: ast + sink(*global_array_static_indirect_1); // clean + indirect_sink(global_array_static); // clean + indirect_sink(global_array_static_indirect_1); // clean + indirect_sink(global_array_static_indirect_2); // $ ir MISSING: ast + } + + static const char* global_pointer_static = "source"; + static const char* global_pointer_static_indirect_1 = "indirect_source(1)"; + static const char* global_pointer_static_indirect_2 = "indirect_source(2)"; + + void test8() { + sink(global_pointer_static); // $ ir MISSING: ast + sink(global_pointer_static_indirect_1); // clean + indirect_sink(global_pointer_static_indirect_1); // $ ir MISSING: ast + sink(global_pointer_static_indirect_2); // clean: global_pointer_static_indirect_2 does not have 2 indirections + indirect_sink(global_pointer_static_indirect_2); // clean: global_pointer_static_indirect_2 does not have 2 indirections + } } \ No newline at end of file