C++: Change queries to use 'asExpr' instead of 'asConvertedExpr'.

MathiasVP · MathiasVP · commit 20f501d1c777 · 2023-09-01T15:01:32.000+01:00
diff --git a/cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/AllocationToInvalidPointer.qll b/cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/AllocationToInvalidPointer.qll
@@ -72,7 +72,7 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
     // Compute `delta` as the constant difference between `x` and `x + 1`.
     bounded1(any(Instruction instr | instr.getUnconvertedResultExpression() = size),
       any(LoadInstruction load | load.getUnconvertedResultExpression() = va), delta) and
-    n.asConvertedExpr() = va.getFullyConverted() and
+    n.asExpr() = va and
     state = delta
   )
 }
@@ -210,7 +210,7 @@ private module InterestingPointerAddInstruction {
     predicate isSource(DataFlow::Node source) {
       // The sources is the same as in the sources for the second
       // projection in the `AllocToInvalidPointerConfig` module.
-      hasSize(source.asConvertedExpr(), _, _)
+      hasSize(source.asExpr(), _, _)
     }
 
     int fieldFlowBranchLimit() { result = allocationToInvalidPointerFieldFlowBranchLimit() }
@@ -243,7 +243,7 @@ private module InterestingPointerAddInstruction {
    */
   predicate isInterestingSize(DataFlow::Node n) {
     exists(DataFlow::Node alloc |
-      hasSize(alloc.asConvertedExpr(), n, _) and
+      hasSize(alloc.asExpr(), n, _) and
       flow(alloc, _)
     )
   }
@@ -268,7 +268,7 @@ private module Config implements ProductFlow::StateConfigSig {
     // we use `state2` to remember that there was an offset (in this case an offset of `1`) added
     // to the size of the allocation. This state is then checked in `isSinkPair`.
     exists(unit) and
-    hasSize(allocSource.asConvertedExpr(), sizeSource, sizeAddend)
+    hasSize(allocSource.asExpr(), sizeSource, sizeAddend)
   }
 
   int fieldFlowBranchLimit1() { result = allocationToInvalidPointerFieldFlowBranchLimit() }
diff --git a/cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql b/cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql
@@ -30,7 +30,7 @@ Expr asSinkExpr(DataFlow::Node node) {
   result = node.asIndirectArgument()
   or
   // We want the conversion so we only get one node for the expression
-  result = node.asConvertedExpr()
+  result = node.asExpr()
 }
 
 module SqlTaintedConfig implements DataFlow::ConfigSig {
diff --git a/cpp/ql/src/Security/CWE/CWE-119/OverrunWriteProductFlow.ql b/cpp/ql/src/Security/CWE/CWE-119/OverrunWriteProductFlow.ql
@@ -38,7 +38,7 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
     // Compute `delta` as the constant difference between `x` and `x + 1`.
     bounded(any(Instruction instr | instr.getUnconvertedResultExpression() = size),
       any(LoadInstruction load | load.getUnconvertedResultExpression() = va), delta) and
-    n.asConvertedExpr() = va.getFullyConverted() and
+    n.asExpr() = va and
     state = delta
   )
 }
@@ -213,7 +213,7 @@ module StringSizeConfig implements ProductFlow::StateConfigSig {
     // we use `state2` to remember that there was an offset (in this case an offset of `1`) added
     // to the size of the allocation. This state is then checked in `isSinkPair`.
     exists(state1) and
-    hasSize(bufSource.asConvertedExpr(), sizeSource, state2) and
+    hasSize(bufSource.asExpr(), sizeSource, state2) and
     validState(sizeSource, state2)
   }
 
diff --git a/cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql b/cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql
@@ -26,7 +26,7 @@ import TaintedAllocationSize::PathGraph
  * taint sink.
  */
 predicate allocSink(HeuristicAllocationExpr alloc, DataFlow::Node sink) {
-  exists(Expr e | e = sink.asConvertedExpr() |
+  exists(Expr e | e = sink.asExpr() |
     e = alloc.getAChild() and
     e.getUnspecifiedType() instanceof IntegralType
   )
diff --git a/cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql b/cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql
@@ -206,25 +206,22 @@ class Encrypted extends Expr {
  * operation `nsr`.
  */
 predicate isSinkSendRecv(DataFlow::Node sink, NetworkSendRecv nsr) {
-  [sink.asIndirectConvertedExpr(), sink.asConvertedExpr()] = nsr.getDataExpr().getFullyConverted()
+  [sink.asIndirectExpr(), sink.asExpr()] = nsr.getDataExpr()
 }
 
 /**
  * Holds if `sink` is a node that is encrypted by `enc`.
  */
-predicate isSinkEncrypt(DataFlow::Node sink, Encrypted enc) {
-  sink.asConvertedExpr() = enc.getFullyConverted()
-}
+predicate isSinkEncrypt(DataFlow::Node sink, Encrypted enc) { sink.asExpr() = enc }
 
 /**
  * Holds if `source` represents a use of a sensitive variable, or data returned by a
  * function returning sensitive data.
  */
 predicate isSourceImpl(DataFlow::Node source) {
-  exists(Expr e |
-    e = source.asConvertedExpr() and
-    e.getUnconverted().(VariableAccess).getTarget() instanceof SourceVariable and
-    not e.hasConversion()
+  exists(VariableAccess e |
+    e = source.asExpr() and
+    e.getTarget() instanceof SourceVariable
   )
   or
   source.asExpr().(FunctionCall).getTarget() instanceof SourceFunction
diff --git a/cpp/ql/src/Security/CWE/CWE-497/SystemData.qll b/cpp/ql/src/Security/CWE/CWE-497/SystemData.qll
@@ -34,7 +34,7 @@ class EnvData extends SystemData {
         .regexpMatch(".*(user|host|admin|root|home|path|http|ssl|snmp|sock|port|proxy|pass|token|crypt|key).*")
   }
 
-  override DataFlow::Node getAnExpr() { result.asIndirectConvertedExpr() = this }
+  override DataFlow::Node getAnExpr() { result.asIndirectExpr() = this }
 
   override predicate isSensitive() {
     this.(EnvironmentRead)
@@ -50,7 +50,7 @@ class EnvData extends SystemData {
 class SqlClientInfo extends SystemData {
   SqlClientInfo() { this.(FunctionCall).getTarget().hasName("mysql_get_client_info") }
 
-  override DataFlow::Node getAnExpr() { result.asIndirectConvertedExpr() = this }
+  override DataFlow::Node getAnExpr() { result.asIndirectExpr() = this }
 
   override predicate isSensitive() { any() }
 }
diff --git a/cpp/ql/src/Security/CWE/CWE-611/Xerces.qll b/cpp/ql/src/Security/CWE/CWE-611/Xerces.qll
@@ -70,7 +70,7 @@ class XercesDomParserLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `AbstractDOMParser.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof AbstractDomParserClass and
-      call.getQualifier() = node.asIndirectConvertedExpr()
+      call.getQualifier() = node.asIndirectExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
@@ -114,7 +114,7 @@ class CreateLSParserLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `DOMLSParserClass.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof DomLSParserClass and
-      call.getQualifier() = node.asIndirectConvertedExpr()
+      call.getQualifier() = node.asIndirectExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
@@ -155,7 +155,7 @@ class SaxParserLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `SAXParser.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof SaxParserClass and
-      call.getQualifier() = node.asIndirectConvertedExpr()
+      call.getQualifier() = node.asIndirectExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
@@ -192,7 +192,7 @@ class Sax2XmlReaderLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `SAX2XMLReader.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof Sax2XmlReader and
-      call.getQualifier() = node.asIndirectConvertedExpr()
+      call.getQualifier() = node.asIndirectExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/tainted.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/tainted.expected
@@ -1,6 +1,4 @@
 WARNING: Module TaintedWithPath has been deprecated and may be removed in future (tainted.ql:9,8-47)
 WARNING: Predicate tainted has been deprecated and may be removed in future (tainted.ql:20,49-74)
 testFailures
-| test_diff.cpp:100:10:100:14 | * ... | Unexpected result: ir-sink=98:17 |
-| test_diff.cpp:102:26:102:30 | (const char *)... | Unexpected result: ir-path=98:17 |
 failures
diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/test_diff.cpp b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/test_diff.cpp
@@ -97,9 +97,9 @@ int main(int argc, char *argv[]) {
 
     char*** p = &argv; // $ ast,ir-path
 
-    sink(*p[0]); // $ ast ir-sink=96:26 ir-sink=98:18
+    sink(*p[0]); // $ ast ir-sink=96:26 ir-sink=98:18 ir-sink=98:17
 
-    calls_sink_with_argv(*p[i]); // $ ir-path=96:26 ir-path=98:18 MISSING:ast
+    calls_sink_with_argv(*p[i]); // $ ir-path=96:26 ir-path=98:18 ir-path=98:17 MISSING:ast
 
     sink(*(argv + 1)); // $ ast ir-path ir-sink
 
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -1,5 +1,2 @@
 testFailures
-| realistic.cpp:61:14:61:55 | bufferLen | Unexpected result: ir= |
-| realistic.cpp:61:59:61:84 | // $ ast ir=53:47 ir=53:55 | Missing result:ir=53:47 |
-| realistic.cpp:61:59:61:84 | // $ ast ir=53:47 ir=53:55 | Missing result:ir=53:55 |
 failures
diff --git a/cpp/ql/test/library-tests/dataflow/fields/realistic.cpp b/cpp/ql/test/library-tests/dataflow/fields/realistic.cpp
@@ -58,7 +58,7 @@ int main(int argc, char** argv) {
             return -1;
         }
         memcpy(dst, foo.bar[i].baz->userInput.buffer, foo.bar[i].baz->userInput.bufferLen);
-        sink((void*)foo.bar[i].baz->userInput.bufferLen); // $ ast ir=53:47 ir=53:55
+        sink((void*)foo.bar[i].baz->userInput.bufferLen); // $ ast ir
         // There is no flow to the following two `sink` calls because the
         // source is the _pointer_ returned by `user_input` rather than the
         // _data_ to which it points.
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp
@@ -165,9 +165,9 @@ void test_map()
 	// array-like access
 	std::map<char *, char *> m10, m11, m12, m13;
 	sink(m10["abc"] = "def");
-	sink(m11["abc"] = source()); // $ ast,ir
+	sink(m11["abc"] = source()); // $ ast ir=168:7 ir=168:20
 	sink(m12.at("abc") = "def");
-	sink(m13.at("abc") = source()); // $ ast,ir
+	sink(m13.at("abc") = source()); // $ ast ir=170:7 ir=170:23
 	sink(m10["abc"]);
 	sink(m11["abc"]); // $ ast,ir
 	sink(m12["abc"]);
@@ -317,9 +317,9 @@ void test_unordered_map()
 	// array-like access
 	std::unordered_map<char *, char *> m10, m11, m12, m13;
 	sink(m10["abc"] = "def");
-	sink(m11["abc"] = source()); // $ ast,ir
+	sink(m11["abc"] = source()); // $ ast ir=320:7 ir=320:20
 	sink(m12.at("abc") = "def");
-	sink(m13.at("abc") = source()); // $ ast,ir
+	sink(m13.at("abc") = source()); // $ ast ir=322:7 ir=322:23
 	sink(m10["abc"]);
 	sink(m11["abc"]); // $ ast,ir
 	sink(m12["abc"]);
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
@@ -13,8 +13,8 @@ void arithAssignments(int source1, int clean1) {
   source1++;
   ++source1;
   source1 += 1;
-  sink(source1); // $ ast,ir
-  sink(++source1); // $ ast,ir
+  sink(source1); // $ ast ir=12:13 ir=12:22
+  sink(++source1); // $ ast ir=12:13 ir=12:22
 }
 
 // --- globals ---
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
@@ -4,30 +4,4 @@ WARNING: Module DataFlow has been deprecated and may be removed in future (taint
 WARNING: Module DataFlow has been deprecated and may be removed in future (taint.ql:68,25-33)
 WARNING: Module TaintTracking has been deprecated and may be removed in future (taint.ql:73,20-33)
 testFailures
-| map.cpp:168:7:168:27 | ... = ... | Unexpected result: ir=168:7 |
-| map.cpp:168:7:168:27 | ... = ... | Unexpected result: ir=168:20 |
-| map.cpp:168:7:168:27 | ... = ... indirection | Unexpected result: ir=168:7 |
-| map.cpp:168:7:168:27 | ... = ... indirection | Unexpected result: ir=168:20 |
-| map.cpp:168:31:168:41 | // $ ast,ir | Missing result:ir= |
-| map.cpp:170:7:170:30 | ... = ... | Unexpected result: ir=170:7 |
-| map.cpp:170:7:170:30 | ... = ... | Unexpected result: ir=170:23 |
-| map.cpp:170:7:170:30 | ... = ... indirection | Unexpected result: ir=170:7 |
-| map.cpp:170:7:170:30 | ... = ... indirection | Unexpected result: ir=170:23 |
-| map.cpp:170:34:170:44 | // $ ast,ir | Missing result:ir= |
-| map.cpp:320:7:320:27 | ... = ... | Unexpected result: ir=320:7 |
-| map.cpp:320:7:320:27 | ... = ... | Unexpected result: ir=320:20 |
-| map.cpp:320:7:320:27 | ... = ... indirection | Unexpected result: ir=320:7 |
-| map.cpp:320:7:320:27 | ... = ... indirection | Unexpected result: ir=320:20 |
-| map.cpp:320:31:320:41 | // $ ast,ir | Missing result:ir= |
-| map.cpp:322:7:322:30 | ... = ... | Unexpected result: ir=322:7 |
-| map.cpp:322:7:322:30 | ... = ... | Unexpected result: ir=322:23 |
-| map.cpp:322:7:322:30 | ... = ... indirection | Unexpected result: ir=322:7 |
-| map.cpp:322:7:322:30 | ... = ... indirection | Unexpected result: ir=322:23 |
-| map.cpp:322:34:322:44 | // $ ast,ir | Missing result:ir= |
-| taint.cpp:16:8:16:14 | source1 | Unexpected result: ir=12:13 |
-| taint.cpp:16:8:16:14 | source1 | Unexpected result: ir=12:22 |
-| taint.cpp:16:18:16:28 | // $ ast,ir | Missing result:ir= |
-| taint.cpp:17:8:17:16 | ++ ... | Unexpected result: ir=12:13 |
-| taint.cpp:17:8:17:16 | ++ ... | Unexpected result: ir=12:22 |
-| taint.cpp:17:20:17:30 | // $ ast,ir | Missing result:ir= |
 failures

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {`
`72`	`72`	// Compute `delta` as the constant difference between `x` and `x + 1`.
`73`	`73`	`bounded1(any(Instruction instr \| instr.getUnconvertedResultExpression() = size),`
`74`	`74`	`any(LoadInstruction load \| load.getUnconvertedResultExpression() = va), delta) and`
`75`		`- n.asConvertedExpr() = va.getFullyConverted() and`
	`75`	`+ n.asExpr() = va and`
`76`	`76`	`state = delta`
`77`	`77`	`)`
`78`	`78`	`}`
`@@ -210,7 +210,7 @@ private module InterestingPointerAddInstruction {`
`210`	`210`	`predicate isSource(DataFlow::Node source) {`
`211`	`211`	`// The sources is the same as in the sources for the second`
`212`	`212`	// projection in the `AllocToInvalidPointerConfig` module.
`213`		`- hasSize(source.asConvertedExpr(), _, _)`
	`213`	`+ hasSize(source.asExpr(), _, _)`
`214`	`214`	`}`
`215`	`215`
`216`	`216`	`int fieldFlowBranchLimit() { result = allocationToInvalidPointerFieldFlowBranchLimit() }`
`@@ -243,7 +243,7 @@ private module InterestingPointerAddInstruction {`
`243`	`243`	`*/`
`244`	`244`	`predicate isInterestingSize(DataFlow::Node n) {`
`245`	`245`	`exists(DataFlow::Node alloc \|`
`246`		`- hasSize(alloc.asConvertedExpr(), n, _) and`
	`246`	`+ hasSize(alloc.asExpr(), n, _) and`
`247`	`247`	`flow(alloc, _)`
`248`	`248`	`)`
`249`	`249`	`}`
`@@ -268,7 +268,7 @@ private module Config implements ProductFlow::StateConfigSig {`
`268`	`268`	// we use `state2` to remember that there was an offset (in this case an offset of `1`) added
`269`	`269`	// to the size of the allocation. This state is then checked in `isSinkPair`.
`270`	`270`	`exists(unit) and`
`271`		`- hasSize(allocSource.asConvertedExpr(), sizeSource, sizeAddend)`
	`271`	`+ hasSize(allocSource.asExpr(), sizeSource, sizeAddend)`
`272`	`272`	`}`
`273`	`273`
`274`	`274`	`int fieldFlowBranchLimit1() { result = allocationToInvalidPointerFieldFlowBranchLimit() }`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ Expr asSinkExpr(DataFlow::Node node) {`
`30`	`30`	`result = node.asIndirectArgument()`
`31`	`31`	`or`
`32`	`32`	`// We want the conversion so we only get one node for the expression`
`33`		`- result = node.asConvertedExpr()`
	`33`	`+ result = node.asExpr()`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`module SqlTaintedConfig implements DataFlow::ConfigSig {`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {`
`38`	`38`	// Compute `delta` as the constant difference between `x` and `x + 1`.
`39`	`39`	`bounded(any(Instruction instr \| instr.getUnconvertedResultExpression() = size),`
`40`	`40`	`any(LoadInstruction load \| load.getUnconvertedResultExpression() = va), delta) and`
`41`		`- n.asConvertedExpr() = va.getFullyConverted() and`
	`41`	`+ n.asExpr() = va and`
`42`	`42`	`state = delta`
`43`	`43`	`)`
`44`	`44`	`}`
`@@ -213,7 +213,7 @@ module StringSizeConfig implements ProductFlow::StateConfigSig {`
`213`	`213`	// we use `state2` to remember that there was an offset (in this case an offset of `1`) added
`214`	`214`	// to the size of the allocation. This state is then checked in `isSinkPair`.
`215`	`215`	`exists(state1) and`
`216`		`- hasSize(bufSource.asConvertedExpr(), sizeSource, state2) and`
	`216`	`+ hasSize(bufSource.asExpr(), sizeSource, state2) and`
`217`	`217`	`validState(sizeSource, state2)`
`218`	`218`	`}`
`219`	`219`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ import TaintedAllocationSize::PathGraph`
`26`	`26`	`* taint sink.`
`27`	`27`	`*/`
`28`	`28`	`predicate allocSink(HeuristicAllocationExpr alloc, DataFlow::Node sink) {`
`29`		`- exists(Expr e \| e = sink.asConvertedExpr() \|`
	`29`	`+ exists(Expr e \| e = sink.asExpr() \|`
`30`	`30`	`e = alloc.getAChild() and`
`31`	`31`	`e.getUnspecifiedType() instanceof IntegralType`
`32`	`32`	`)`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ class EnvData extends SystemData {`
`34`	`34`	`.regexpMatch(".(user\|host\|admin\|root\|home\|path\|http\|ssl\|snmp\|sock\|port\|proxy\|pass\|token\|crypt\|key).")`
`35`	`35`	`}`
`36`	`36`
`37`		`- override DataFlow::Node getAnExpr() { result.asIndirectConvertedExpr() = this }`
	`37`	`+ override DataFlow::Node getAnExpr() { result.asIndirectExpr() = this }`
`38`	`38`
`39`	`39`	`override predicate isSensitive() {`
`40`	`40`	`this.(EnvironmentRead)`
`@@ -50,7 +50,7 @@ class EnvData extends SystemData {`
`50`	`50`	`class SqlClientInfo extends SystemData {`
`51`	`51`	`SqlClientInfo() { this.(FunctionCall).getTarget().hasName("mysql_get_client_info") }`
`52`	`52`
`53`		`- override DataFlow::Node getAnExpr() { result.asIndirectConvertedExpr() = this }`
	`53`	`+ override DataFlow::Node getAnExpr() { result.asIndirectExpr() = this }`
`54`	`54`
`55`	`55`	`override predicate isSensitive() { any() }`
`56`	`56`	`}`