Merge pull request #14151 from MathiasVP/deduplicate-dataflow-results-take-3

C++: Deduplicate dataflow query results

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -1956,9 +1956,7 @@ class TranslatedNonConstantAllocationSize extends TranslatedAllocationSize {
     result = this.getExtent().getResult()
   }
 
-  private TranslatedExpr getExtent() {
-    result = getTranslatedExpr(expr.getExtent().getFullyConverted())
-  }
+  TranslatedExpr getExtent() { result = getTranslatedExpr(expr.getExtent().getFullyConverted()) }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.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() }

cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/AllocationToInvalidPointer.qll

@@ -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 {

cpp/ql/src/Security/CWE/CWE-089/SqlTainted.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)
   }
 

cpp/ql/src/Security/CWE/CWE-119/OverrunWriteProductFlow.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
   )

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.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

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

@@ -33,14 +33,6 @@ module ExposedSystemDataConfig implements DataFlow::ConfigSig {
 module ExposedSystemData = TaintTracking::Global<ExposedSystemDataConfig>;
 
 from ExposedSystemData::PathNode source, ExposedSystemData::PathNode sink
-where
-  ExposedSystemData::flowPath(source, sink) and
-  not exists(
-    DataFlow::Node alt // remove duplicate results on conversions
-  |
-    ExposedSystemData::flow(source.getNode(), alt) and
-    alt.asConvertedExpr() = sink.getNode().asIndirectExpr() and
-    alt != sink.getNode()
-  )
+where ExposedSystemData::flowPath(source, sink)
 select sink, source, sink, "This operation exposes system data from $@.", source,
   source.getNode().toString()

cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql

@@ -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() }
 }

cpp/ql/src/Security/CWE/CWE-497/SystemData.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