C++: Restore lost result on git/git. We lost the result in a00bd7a because the added check for type T to type T* conversion didn't handle const qualifiers.

MathiasVP · MathiasVP · commit b4f9b1590d04 · 2021-01-22T14:20:18.000+01:00
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
@@ -451,7 +451,7 @@ Type getResultTypeOfSourceValue(CopyValueInstruction copy) {
  * pops the `ArrayContent` off the access path when a value-to-pointer or value-to-reference conversion
  * happens on the argument that is ends up as the target of such a store.
  */
-private predicate innerReadSteap(Node node1, Content a, Node node2) {
+private predicate innerReadStep(Node node1, Content a, Node node2) {
   a = TArrayContent() and
   exists(WriteSideEffectInstruction write, CallInstruction call, CopyValueInstruction copyValue |
     write.getPrimaryInstruction() = call and
@@ -463,8 +463,9 @@ private predicate innerReadSteap(Node node1, Content a, Node node2) {
     ) and
     node2.asInstruction() = write and
     copyValue = call.getArgument(write.getIndex()) and
-    [getPointerType(copyValue).getBaseType(), getReferenceType(copyValue).getBaseType()] =
-      getResultTypeOfSourceValue(copyValue)
+    // Check that `copyValue` is actually doing a T to a T* conversion.
+    [getPointerType(copyValue).getBaseType(), getReferenceType(copyValue).getBaseType()].stripType() =
+      getResultTypeOfSourceValue(copyValue).stripType()
   )
 }
 
@@ -477,7 +478,7 @@ predicate readStep(Node node1, Content f, Node node2) {
   aliasedReadStep(node1, f, node2) or
   arrayReadStep(node1, f, node2) or
   instrToFieldNodeReadStep(node1, f, node2) or
-  innerReadSteap(node1, f, node2)
+  innerReadStep(node1, f, node2)
 }
 
 /**
diff --git a/cpp/ql/test/library-tests/dataflow/fields/by_reference.cpp b/cpp/ql/test/library-tests/dataflow/fields/by_reference.cpp
@@ -1,4 +1,4 @@
-void sink(void *o);
+void sink(void *o); void sink(const char *o);
 void *user_input(void);
 
 struct S {
@@ -135,3 +135,13 @@ void test_outer_with_ref(Outer *pouter) {
   sink(pouter->inner_ptr->a); // $ ast MISSING: ir
   sink(pouter->a); // $ ast,ir
 }
+
+void taint_a_ptr(const char **pa) {
+  *pa = (char*)user_input();
+}
+
+void test_const_char_ref() {
+  const char* s;
+  taint_a_ptr(&s);
+  sink(s); // $ ast ir=140:9 ir=140:16
+}
diff --git a/cpp/ql/test/library-tests/dataflow/fields/dataflow-consistency.expected b/cpp/ql/test/library-tests/dataflow/fields/dataflow-consistency.expected
@@ -123,6 +123,9 @@ postWithInFlow
 | by_reference.cpp:108:24:108:24 | a [inner post update] | PostUpdateNode should not be the target of local flow. |
 | by_reference.cpp:123:28:123:36 | inner_ptr [inner post update] | PostUpdateNode should not be the target of local flow. |
 | by_reference.cpp:127:30:127:38 | inner_ptr [inner post update] | PostUpdateNode should not be the target of local flow. |
+| by_reference.cpp:140:3:140:5 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| by_reference.cpp:140:4:140:5 | pa [inner post update] | PostUpdateNode should not be the target of local flow. |
+| by_reference.cpp:145:16:145:16 | s [inner post update] | PostUpdateNode should not be the target of local flow. |
 | complex.cpp:11:22:11:23 | a_ [post update] | PostUpdateNode should not be the target of local flow. |
 | complex.cpp:12:22:12:23 | b_ [post update] | PostUpdateNode should not be the target of local flow. |
 | conflated.cpp:10:3:10:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected
@@ -195,6 +195,14 @@ edges
 | by_reference.cpp:134:16:134:27 | inner_nested [a, a] | by_reference.cpp:134:29:134:29 | a [a] |
 | by_reference.cpp:134:29:134:29 | a [a] | by_reference.cpp:134:29:134:29 | a |
 | by_reference.cpp:136:16:136:16 | a [a] | by_reference.cpp:136:16:136:16 | a |
+| by_reference.cpp:140:3:140:27 | Chi [array content] | by_reference.cpp:145:15:145:16 | taint_a_ptr output argument [array content] |
+| by_reference.cpp:140:3:140:27 | ChiTotal [post update] [array content] | by_reference.cpp:140:3:140:27 | Chi [array content] |
+| by_reference.cpp:140:3:140:27 | ChiTotal [post update] [array content] | by_reference.cpp:145:15:145:16 | taint_a_ptr output argument [array content] |
+| by_reference.cpp:140:9:140:27 | (char *)... | by_reference.cpp:140:3:140:27 | ChiTotal [post update] [array content] |
+| by_reference.cpp:140:9:140:27 | (const char *)... | by_reference.cpp:140:3:140:27 | ChiTotal [post update] [array content] |
+| by_reference.cpp:140:16:140:25 | call to user_input | by_reference.cpp:140:3:140:27 | ChiTotal [post update] [array content] |
+| by_reference.cpp:145:15:145:16 | taint_a_ptr output argument | by_reference.cpp:146:8:146:8 | s |
+| by_reference.cpp:145:15:145:16 | taint_a_ptr output argument [array content] | by_reference.cpp:145:15:145:16 | taint_a_ptr output argument |
 | complex.cpp:40:17:40:17 | *b [a_] | complex.cpp:42:18:42:18 | call to a |
 | complex.cpp:40:17:40:17 | *b [b_] | complex.cpp:42:16:42:16 | a output argument [b_] |
 | complex.cpp:40:17:40:17 | *b [b_] | complex.cpp:43:18:43:18 | call to b |
@@ -504,6 +512,14 @@ nodes
 | by_reference.cpp:134:29:134:29 | a [a] | semmle.label | a [a] |
 | by_reference.cpp:136:16:136:16 | a | semmle.label | a |
 | by_reference.cpp:136:16:136:16 | a [a] | semmle.label | a [a] |
+| by_reference.cpp:140:3:140:27 | Chi [array content] | semmle.label | Chi [array content] |
+| by_reference.cpp:140:3:140:27 | ChiTotal [post update] [array content] | semmle.label | ChiTotal [post update] [array content] |
+| by_reference.cpp:140:9:140:27 | (char *)... | semmle.label | (char *)... |
+| by_reference.cpp:140:9:140:27 | (const char *)... | semmle.label | (const char *)... |
+| by_reference.cpp:140:16:140:25 | call to user_input | semmle.label | call to user_input |
+| by_reference.cpp:145:15:145:16 | taint_a_ptr output argument | semmle.label | taint_a_ptr output argument |
+| by_reference.cpp:145:15:145:16 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
+| by_reference.cpp:146:8:146:8 | s | semmle.label | s |
 | complex.cpp:40:17:40:17 | *b [a_] | semmle.label | *b [a_] |
 | complex.cpp:40:17:40:17 | *b [b_] | semmle.label | *b [b_] |
 | complex.cpp:40:17:40:17 | *b [f, f, a_] | semmle.label | *b [f, f, a_] |
@@ -649,6 +665,9 @@ nodes
 | by_reference.cpp:132:14:132:14 | a | by_reference.cpp:96:8:96:17 | call to user_input | by_reference.cpp:132:14:132:14 | a | a flows from $@ | by_reference.cpp:96:8:96:17 | call to user_input | call to user_input |
 | by_reference.cpp:134:29:134:29 | a | by_reference.cpp:88:13:88:22 | call to user_input | by_reference.cpp:134:29:134:29 | a | a flows from $@ | by_reference.cpp:88:13:88:22 | call to user_input | call to user_input |
 | by_reference.cpp:136:16:136:16 | a | by_reference.cpp:96:8:96:17 | call to user_input | by_reference.cpp:136:16:136:16 | a | a flows from $@ | by_reference.cpp:96:8:96:17 | call to user_input | call to user_input |
+| by_reference.cpp:146:8:146:8 | s | by_reference.cpp:140:9:140:27 | (char *)... | by_reference.cpp:146:8:146:8 | s | s flows from $@ | by_reference.cpp:140:9:140:27 | (char *)... | (char *)... |
+| by_reference.cpp:146:8:146:8 | s | by_reference.cpp:140:9:140:27 | (const char *)... | by_reference.cpp:146:8:146:8 | s | s flows from $@ | by_reference.cpp:140:9:140:27 | (const char *)... | (const char *)... |
+| by_reference.cpp:146:8:146:8 | s | by_reference.cpp:140:16:140:25 | call to user_input | by_reference.cpp:146:8:146:8 | s | s flows from $@ | by_reference.cpp:140:16:140:25 | call to user_input | call to user_input |
 | complex.cpp:42:18:42:18 | call to a | complex.cpp:53:19:53:28 | call to user_input | complex.cpp:42:18:42:18 | call to a | call to a flows from $@ | complex.cpp:53:19:53:28 | call to user_input | call to user_input |
 | complex.cpp:42:18:42:18 | call to a | complex.cpp:55:19:55:28 | call to user_input | complex.cpp:42:18:42:18 | call to a | call to a flows from $@ | complex.cpp:55:19:55:28 | call to user_input | call to user_input |
 | complex.cpp:43:18:43:18 | call to b | complex.cpp:54:19:54:28 | call to user_input | complex.cpp:43:18:43:18 | call to b | call to b flows from $@ | complex.cpp:54:19:54:28 | call to user_input | call to user_input |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/partial-definition-diff.expected b/cpp/ql/test/library-tests/dataflow/fields/partial-definition-diff.expected
@@ -242,6 +242,8 @@
 | by_reference.cpp:134:29:134:29 | a | AST only |
 | by_reference.cpp:135:27:135:27 | a | AST only |
 | by_reference.cpp:136:16:136:16 | a | AST only |
+| by_reference.cpp:140:3:140:5 | * ... | AST only |
+| by_reference.cpp:145:15:145:16 | & ... | AST only |
 | complex.cpp:9:20:9:21 | this | IR only |
 | complex.cpp:10:20:10:21 | this | IR only |
 | complex.cpp:11:22:11:23 | a_ | AST only |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/partial-definition.expected b/cpp/ql/test/library-tests/dataflow/fields/partial-definition.expected
@@ -352,6 +352,8 @@
 | by_reference.cpp:135:27:135:27 | a |
 | by_reference.cpp:136:8:136:13 | pouter |
 | by_reference.cpp:136:16:136:16 | a |
+| by_reference.cpp:140:3:140:5 | * ... |
+| by_reference.cpp:145:15:145:16 | & ... |
 | complex.cpp:11:22:11:23 | a_ |
 | complex.cpp:11:22:11:23 | this |
 | complex.cpp:12:22:12:23 | b_ |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/path-flow.expected b/cpp/ql/test/library-tests/dataflow/fields/path-flow.expected
@@ -323,6 +323,9 @@ edges
 | by_reference.cpp:135:8:135:13 | pouter [inner_ptr, a] | by_reference.cpp:135:16:135:24 | inner_ptr [a] |
 | by_reference.cpp:135:16:135:24 | inner_ptr [a] | by_reference.cpp:135:27:135:27 | a |
 | by_reference.cpp:136:8:136:13 | pouter [a] | by_reference.cpp:136:16:136:16 | a |
+| by_reference.cpp:140:4:140:5 | pa [inner post update] | by_reference.cpp:145:15:145:16 | ref arg & ... |
+| by_reference.cpp:140:16:140:25 | call to user_input | by_reference.cpp:140:4:140:5 | pa [inner post update] |
+| by_reference.cpp:145:15:145:16 | ref arg & ... | by_reference.cpp:146:8:146:8 | s |
 | complex.cpp:40:17:40:17 | b [inner, f, a_] | complex.cpp:42:8:42:8 | b [inner, f, a_] |
 | complex.cpp:40:17:40:17 | b [inner, f, b_] | complex.cpp:43:8:43:8 | b [inner, f, b_] |
 | complex.cpp:42:8:42:8 | b [inner, f, a_] | complex.cpp:42:10:42:14 | inner [f, a_] |
@@ -855,6 +858,10 @@ nodes
 | by_reference.cpp:135:27:135:27 | a | semmle.label | a |
 | by_reference.cpp:136:8:136:13 | pouter [a] | semmle.label | pouter [a] |
 | by_reference.cpp:136:16:136:16 | a | semmle.label | a |
+| by_reference.cpp:140:4:140:5 | pa [inner post update] | semmle.label | pa [inner post update] |
+| by_reference.cpp:140:16:140:25 | call to user_input | semmle.label | call to user_input |
+| by_reference.cpp:145:15:145:16 | ref arg & ... | semmle.label | ref arg & ... |
+| by_reference.cpp:146:8:146:8 | s | semmle.label | s |
 | complex.cpp:40:17:40:17 | b [inner, f, a_] | semmle.label | b [inner, f, a_] |
 | complex.cpp:40:17:40:17 | b [inner, f, b_] | semmle.label | b [inner, f, b_] |
 | complex.cpp:42:8:42:8 | b [inner, f, a_] | semmle.label | b [inner, f, a_] |
@@ -1117,6 +1124,7 @@ nodes
 | by_reference.cpp:134:29:134:29 | a | by_reference.cpp:88:13:88:22 | call to user_input | by_reference.cpp:134:29:134:29 | a | a flows from $@ | by_reference.cpp:88:13:88:22 | call to user_input | call to user_input |
 | by_reference.cpp:135:27:135:27 | a | by_reference.cpp:88:13:88:22 | call to user_input | by_reference.cpp:135:27:135:27 | a | a flows from $@ | by_reference.cpp:88:13:88:22 | call to user_input | call to user_input |
 | by_reference.cpp:136:16:136:16 | a | by_reference.cpp:96:8:96:17 | call to user_input | by_reference.cpp:136:16:136:16 | a | a flows from $@ | by_reference.cpp:96:8:96:17 | call to user_input | call to user_input |
+| by_reference.cpp:146:8:146:8 | s | by_reference.cpp:140:16:140:25 | call to user_input | by_reference.cpp:146:8:146:8 | s | s flows from $@ | by_reference.cpp:140:16:140:25 | call to user_input | call to user_input |
 | complex.cpp:42:18:42:18 | call to a | complex.cpp:53:19:53:28 | call to user_input | complex.cpp:42:18:42:18 | call to a | call to a flows from $@ | complex.cpp:53:19:53:28 | call to user_input | call to user_input |
 | complex.cpp:42:18:42:18 | call to a | complex.cpp:55:19:55:28 | call to user_input | complex.cpp:42:18:42:18 | call to a | call to a flows from $@ | complex.cpp:55:19:55:28 | call to user_input | call to user_input |
 | complex.cpp:43:18:43:18 | call to b | complex.cpp:54:19:54:28 | call to user_input | complex.cpp:43:18:43:18 | call to b | call to b flows from $@ | complex.cpp:54:19:54:28 | call to user_input | call to user_input |
