C++: Update XXE XML query with DataFlow::ConfigSig

2026-05-14 03:09:26 +02:00 · 2023-03-08 14:57:44 +01:00
parent 6f2407412e
commit 30cbc91092
4 changed files with 68 additions and 62 deletions
--- a/cpp/ql/src/Security/CWE/CWE-611/Libxml2.qll
+++ b/cpp/ql/src/Security/CWE/CWE-611/Libxml2.qll
@@ -48,7 +48,7 @@ class Libxml2BadOption extends EnumConstant {
 class LibXml2Library extends XmlLibrary {
  LibXml2Library() { this = "LibXml2Library" }

-  override predicate configurationSource(DataFlow::Node node, string flowstate) {
+  override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
    // source is an `options` argument on a libxml2 parse call that specifies
    // at least one unsafe option.
    //
@@ -59,7 +59,7 @@ class LibXml2Library extends XmlLibrary {
    exists(Libxml2ParseCall call, Expr options |
      options = call.getOptions() and
      node.asExpr() = options and
-      flowstate = "libxml2" and
+      flowstate instanceof TLibXml2FlowState and
      exists(Libxml2BadOption opt |
        globalValueNumber(options).getAnExpr().getValue().toInt().bitAnd(opt.getValue().toInt()) !=
          0
@@ -67,12 +67,12 @@ class LibXml2Library extends XmlLibrary {
    )
  }

-  override predicate configurationSink(DataFlow::Node node, string flowstate) {
+  override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
    // sink is the `options` argument on a `libxml2` parse call.
    exists(Libxml2ParseCall call, Expr options |
      options = call.getOptions() and
      node.asExpr() = options and
-      flowstate = "libxml2"
+      flowstate instanceof TLibXml2FlowState
    )
  }
 }
--- a/cpp/ql/src/Security/CWE/CWE-611/XML.qll
+++ b/cpp/ql/src/Security/CWE/CWE-611/XML.qll
@@ -11,10 +11,20 @@ import Libxml2
 /**
 * A flow state representing a possible configuration of an XML object.
 */
-abstract class XxeFlowState extends DataFlow::FlowState {
-  bindingset[this]
-  XxeFlowState() { any() } // required characteristic predicate
-}
+newtype TXxeFlowState =
+  /**
+   * Flow state for `AbstractDOMParser` or `SAXParser`, where:
+   *  - `disabledDefaultEntityResolution` is 1 if `setDisableDefaultEntityResolution`
+   *    is `true`, 0 otherwise.
+   *  - `createEntityReferenceNodes` is 1 if `setCreateEntityReferenceNodes`
+   *    is `true`, 0 otherwise.
+   */
+  TXercesFlowState(int disabledDefaultEntityResolution, int createEntityReferenceNodes) {
+    disabledDefaultEntityResolution in [0, 1] and
+    createEntityReferenceNodes in [0, 1]
+  } or
+  /** Flow state for libxml2 */
+  TLibXml2FlowState()

 /**
 * An XML library or interface.
@@ -28,13 +38,13 @@ abstract class XmlLibrary extends string {
   * object for this XML library, along with `flowstate` representing its
   * initial state.
   */
-  abstract predicate configurationSource(DataFlow::Node node, string flowstate);
+  abstract predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate);

  /**
   * Holds if `node` is  the sink node where an unsafe configuration object is
   * used to interpret XML.
   */
-  abstract predicate configurationSink(DataFlow::Node node, string flowstate);
+  abstract predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate);
 }

 /**
@@ -51,5 +61,5 @@ abstract class XxeFlowStateTransformer extends Expr {
   * transform(transform(x)) = transform(x)
   * ```
   */
-  abstract XxeFlowState transform(XxeFlowState flowstate);
+  abstract TXxeFlowState transform(TXxeFlowState flowstate);
 }
--- a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql
+++ b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql
@@ -14,38 +14,40 @@

 import cpp
 import XML
-import DataFlow::PathGraph
+import XxeFlow::PathGraph

 /**
 * A configuration for tracking XML objects and their states.
 */
-class XxeConfiguration extends DataFlow::Configuration {
-  XxeConfiguration() { this = "XXEConfiguration" }
+module XxeConfiguration implements DataFlow::StateConfigSig {
+  class FlowState = TXxeFlowState;

-  override predicate isSource(DataFlow::Node node, string flowstate) {
+  predicate isSource(DataFlow::Node node, FlowState flowstate) {
    any(XmlLibrary l).configurationSource(node, flowstate)
  }

-  override predicate isSink(DataFlow::Node node, string flowstate) {
+  predicate isSink(DataFlow::Node node, FlowState flowstate) {
    any(XmlLibrary l).configurationSink(node, flowstate)
  }

-  override predicate isAdditionalFlowStep(
-    DataFlow::Node node1, string state1, DataFlow::Node node2, string state2
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
  ) {
    // create additional flow steps for `XxeFlowStateTransformer`s
    state2 = node2.asIndirectExpr().(XxeFlowStateTransformer).transform(state1) and
    DataFlow::simpleLocalFlowStep(node1, node2)
  }

-  override predicate isBarrier(DataFlow::Node node, string flowstate) {
+  predicate isBarrier(DataFlow::Node node, FlowState flowstate) {
    // when the flowstate is transformed at a call node, block the original
    // flowstate value.
    node.asIndirectExpr().(XxeFlowStateTransformer).transform(flowstate) != flowstate
  }
 }

-from XxeConfiguration conf, DataFlow::PathNode source, DataFlow::PathNode sink
-where conf.hasFlowPath(source, sink)
+module XxeFlow = DataFlow::MakeWithState<XxeConfiguration>;
+
+from XxeFlow::PathNode source, XxeFlow::PathNode sink
+where XxeFlow::hasFlowPath(source, sink)
 select sink, source, sink,
  "This $@ is not configured to prevent an XML external entity (XXE) attack.", source, "XML parser"
--- a/cpp/ql/src/Security/CWE/CWE-611/Xerces.qll
+++ b/cpp/ql/src/Security/CWE/CWE-611/Xerces.qll
@@ -7,39 +7,33 @@ import XML
 import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 import semmle.code.cpp.ir.IR

-/**
- * Gets a valid flow state for `AbstractDOMParser` or `SAXParser` flow.
- *
- * These flow states take the form `Xerces-A-B`, where:
- *  - A is 1 if `setDisableDefaultEntityResolution` is `true`, 0 otherwise.
- *  - B is 1 if `setCreateEntityReferenceNodes` is `true`, 0 otherwise.
- */
-predicate encodeXercesFlowState(
-  string flowstate, int disabledDefaultEntityResolution, int createEntityReferenceNodes
-) {
-  flowstate = "Xerces-0-0" and
-  disabledDefaultEntityResolution = 0 and
-  createEntityReferenceNodes = 0
-  or
-  flowstate = "Xerces-0-1" and
-  disabledDefaultEntityResolution = 0 and
-  createEntityReferenceNodes = 1
-  or
-  flowstate = "Xerces-1-0" and
-  disabledDefaultEntityResolution = 1 and
-  createEntityReferenceNodes = 0
-  or
-  flowstate = "Xerces-1-1" and
-  disabledDefaultEntityResolution = 1 and
-  createEntityReferenceNodes = 1
-}
-
 /**
 * A flow state representing the configuration of an `AbstractDOMParser` or
 * `SAXParser` object.
 */
-class XercesFlowState extends XxeFlowState {
-  XercesFlowState() { encodeXercesFlowState(this, _, _) }
+class XercesFlowState extends TXxeFlowState {
+  int disabledDefaultEntityResolution;
+  int createEntityReferenceNodes;
+
+  XercesFlowState() {
+    this = TXercesFlowState(disabledDefaultEntityResolution, createEntityReferenceNodes)
+  }
+
+  int getDisabledDefaultEntityResolution() { result = disabledDefaultEntityResolution }
+
+  int getCreateEntityReferenceNodes() { result = createEntityReferenceNodes }
+
+  string toString() { result = "XercesFlowState" }
+}
+
+/**
+ * Gets a valid flow state for `AbstractDOMParser` or `SAXParser` flow.
+ */
+predicate encodeXercesFlowState(
+  XercesFlowState flowstate, int disabledDefaultEntityResolution, int createEntityReferenceNodes
+) {
+  flowstate.getDisabledDefaultEntityResolution() = disabledDefaultEntityResolution and
+  flowstate.getCreateEntityReferenceNodes() = createEntityReferenceNodes
 }

 /**
@@ -62,7 +56,7 @@ class XercesDomParserClass extends Class {
 class XercesDomParserLibrary extends XmlLibrary {
  XercesDomParserLibrary() { this = "XercesDomParserLibrary" }

-  override predicate configurationSource(DataFlow::Node node, string flowstate) {
+  override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
    // source is the write on `this` of a call to the `XercesDOMParser`
    // constructor.
    exists(Call call |
@@ -72,7 +66,7 @@ class XercesDomParserLibrary extends XmlLibrary {
    )
  }

-  override predicate configurationSink(DataFlow::Node node, string flowstate) {
+  override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
    // sink is the read of the qualifier of a call to `AbstractDOMParser.parse`.
    exists(Call call |
      call.getTarget().getClassAndName("parse") instanceof AbstractDomParserClass and
@@ -107,7 +101,7 @@ class CreateLSParser extends Function {
 class CreateLSParserLibrary extends XmlLibrary {
  CreateLSParserLibrary() { this = "CreateLSParserLibrary" }

-  override predicate configurationSource(DataFlow::Node node, string flowstate) {
+  override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
    // source is the result of a call to `createLSParser`.
    exists(Call call |
      call.getTarget() instanceof CreateLSParser and
@@ -116,7 +110,7 @@ class CreateLSParserLibrary extends XmlLibrary {
    )
  }

-  override predicate configurationSink(DataFlow::Node node, string flowstate) {
+  override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
    // sink is the read of the qualifier of a call to `DOMLSParserClass.parse`.
    exists(Call call |
      call.getTarget().getClassAndName("parse") instanceof DomLSParserClass and
@@ -147,7 +141,7 @@ class Sax2XmlReader extends Class {
 class SaxParserLibrary extends XmlLibrary {
  SaxParserLibrary() { this = "SaxParserLibrary" }

-  override predicate configurationSource(DataFlow::Node node, string flowstate) {
+  override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
    // source is the write on `this` of a call to the `SAXParser`
    // constructor.
    exists(Call call |
@@ -157,7 +151,7 @@ class SaxParserLibrary extends XmlLibrary {
    )
  }

-  override predicate configurationSink(DataFlow::Node node, string flowstate) {
+  override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
    // sink is the read of the qualifier of a call to `SAXParser.parse`.
    exists(Call call |
      call.getTarget().getClassAndName("parse") instanceof SaxParserClass and
@@ -185,7 +179,7 @@ class CreateXmlReader extends Function {
 class Sax2XmlReaderLibrary extends XmlLibrary {
  Sax2XmlReaderLibrary() { this = "Sax2XmlReaderLibrary" }

-  override predicate configurationSource(DataFlow::Node node, string flowstate) {
+  override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
    // source is the result of a call to `createXMLReader`.
    exists(Call call |
      call.getTarget() instanceof CreateXmlReader and
@@ -194,7 +188,7 @@ class Sax2XmlReaderLibrary extends XmlLibrary {
    )
  }

-  override predicate configurationSink(DataFlow::Node node, string flowstate) {
+  override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
    // sink is the read of the qualifier of a call to `SAX2XMLReader.parse`.
    exists(Call call |
      call.getTarget().getClassAndName("parse") instanceof Sax2XmlReader and
@@ -227,7 +221,7 @@ class DisableDefaultEntityResolutionTransformer extends XxeFlowStateTransformer
    )
  }

-  final override XxeFlowState transform(XxeFlowState flowstate) {
+  final override TXxeFlowState transform(TXxeFlowState flowstate) {
    exists(int createEntityReferenceNodes |
      encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
      (
@@ -258,7 +252,7 @@ class CreateEntityReferenceNodesTransformer extends XxeFlowStateTransformer {
    )
  }

-  final override XxeFlowState transform(XxeFlowState flowstate) {
+  final override TXxeFlowState transform(TXxeFlowState flowstate) {
    exists(int disabledDefaultEntityResolution |
      encodeXercesFlowState(flowstate, disabledDefaultEntityResolution, _) and
      (
@@ -301,7 +295,7 @@ class SetFeatureTransformer extends XxeFlowStateTransformer {
    )
  }

-  final override XxeFlowState transform(XxeFlowState flowstate) {
+  final override TXxeFlowState transform(TXxeFlowState flowstate) {
    exists(int createEntityReferenceNodes |
      encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
      (
@@ -359,7 +353,7 @@ class DomConfigurationSetParameterTransformer extends XxeFlowStateTransformer {
    )
  }

-  final override XxeFlowState transform(XxeFlowState flowstate) {
+  final override TXxeFlowState transform(TXxeFlowState flowstate) {
    exists(int createEntityReferenceNodes |
      encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
      (