XXE: Changes for review

python/ql/src/experimental/Security/CWE-611/SimpleXmlRpcServer.ql

@@ -0,0 +1,25 @@
+/**
+ * @name SimpleXMLRPCServer DoS vulnerability
+ * @description SimpleXMLRPCServer is vulnerable to DoS attacks from untrusted user input
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id py/simple-xml-rpc-server-dos
+ * @tags security
+ *       external/cwe/cwe-776
+ */
+
+private import python
+private import experimental.semmle.python.Concepts
+private import semmle.python.ApiGraphs
+
+from DataFlow::CallCfgNode call, string kinds
+where
+  call = API::moduleImport("xmlrpc").getMember("server").getMember("SimpleXMLRPCServer").getACall() and
+  kinds =
+    strictconcat(XML::XMLVulnerabilityKind kind |
+      kind.isBillionLaughs() or kind.isQuadraticBlowup()
+    |
+      kind, ", "
+    )
+select call, "SimpleXMLRPCServer is vulnerable to: " + kinds + "."

python/ql/src/experimental/Security/CWE-611/XmlEntityInjection.ql

@@ -15,8 +15,17 @@ import python
 import experimental.semmle.python.security.dataflow.XmlEntityInjection
 import DataFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, string kind
-where XmlEntityInjection::xmlEntityInjectionVulnerable(source, sink, kind)
+from
+  XmlEntityInjection::XmlEntityInjectionConfiguration config, DataFlow::PathNode source,
+  DataFlow::PathNode sink, string kinds
+where
+  config.hasFlowPath(source, sink) and
+  kinds =
+    strictconcat(string kind |
+      kind = sink.getNode().(XmlEntityInjection::Sink).getVulnerableKind()
+    |
+      kind, ", "
+    )
 select sink.getNode(), source, sink,
-  "$@ XML input is constructed from a $@ and is vulnerable to " + kind + ".", sink.getNode(),
+  "$@ XML input is constructed from a $@ and is vulnerable to: " + kinds + ".", sink.getNode(),
   "This", source.getNode(), "user-provided value"

python/ql/src/experimental/semmle/python/Concepts.qll

@@ -16,69 +16,53 @@ private import experimental.semmle.python.Frameworks
 
 module XML {
   /**
-   * A data-flow node that collects functions parsing XML.
+   * A kind of XML vulnerability.
    *
-   * Extend this class to model new APIs. If you want to refine existing API models,
-   * extend `XMLParsing` instead.
+   * See https://pypi.org/project/defusedxml/#python-xml-libraries
    */
-  class XMLParsing extends DataFlow::Node instanceof XMLParsing::Range {
-    /**
-     * Gets the argument containing the content to parse.
-     */
-    DataFlow::Node getAnInput() { result = super.getAnInput() }
+  class XMLVulnerabilityKind extends string {
+    XMLVulnerabilityKind() {
+      this in ["Billion Laughs", "Quadratic Blowup", "XXE", "DTD retrieval"]
+    }
 
-    /**
-     * Holds if the parsing method or the parser holding it is vulnerable to `kind`.
-     */
-    predicate vulnerable(string kind) { super.vulnerable(kind) }
-  }
+    /** Holds for Billion Laughs vulnerability kind. */
+    predicate isBillionLaughs() { this = "Billion Laughs" }
 
-  /** Provides classes for modeling XML parsing APIs. */
-  module XMLParsing {
-    /**
-     * A data-flow node that collects functions parsing XML.
-     *
-     * Extend this class to model new APIs. If you want to refine existing API models,
-     * extend `XMLParsing` instead.
-     */
-    abstract class Range extends DataFlow::Node {
-      /**
-       * Gets the argument containing the content to parse.
-       */
-      abstract DataFlow::Node getAnInput();
+    /** Holds for Quadratic Blowup vulnerability kind. */
+    predicate isQuadraticBlowup() { this = "Quadratic Blowup" }
 
-      /**
-       * Holds if the parsing method or the parser holding it is vulnerable to `kind`.
-       */
-      abstract predicate vulnerable(string kind);
-    }
+    /** Holds for XXE vulnerability kind. */
+    predicate isXxe() { this = "XXE" }
+
+    /** Holds for DTD retrieval vulnerability kind. */
+    predicate isDtdRetrieval() { this = "DTD retrieval" }
   }
 
   /**
-   * A data-flow node that collects XML parsers.
+   * A data-flow node that parses XML.
    *
    * Extend this class to model new APIs. If you want to refine existing API models,
-   * extend `XMLParser` instead.
+   * extend `XMLParsing` instead.
    */
-  class XMLParser extends DataFlow::Node instanceof XMLParser::Range {
+  class XMLParsing extends DataFlow::Node instanceof XMLParsing::Range {
     /**
      * Gets the argument containing the content to parse.
      */
     DataFlow::Node getAnInput() { result = super.getAnInput() }
 
     /**
-     * Holds if the parser is vulnerable to `kind`.
+     * Holds if this XML parsing is vulnerable to `kind`.
      */
-    predicate vulnerable(string kind) { super.vulnerable(kind) }
+    predicate vulnerableTo(XMLVulnerabilityKind kind) { super.vulnerableTo(kind) }
   }
 
-  /** Provides classes for modeling XML parsers. */
-  module XMLParser {
+  /** Provides classes for modeling XML parsing APIs. */
+  module XMLParsing {
     /**
-     * A data-flow node that collects XML parsers.
+     * A data-flow node that parses XML.
      *
      * Extend this class to model new APIs. If you want to refine existing API models,
-     * extend `XMLParser` instead.
+     * extend `XMLParsing` instead.
      */
     abstract class Range extends DataFlow::Node {
       /**
@@ -87,9 +71,9 @@ module XML {
       abstract DataFlow::Node getAnInput();
 
       /**
-       * Holds if the parser is vulnerable to `kind`.
+       * Holds if this XML parsing is vulnerable to `kind`.
        */
-      abstract predicate vulnerable(string kind);
+      abstract predicate vulnerableTo(XMLVulnerabilityKind kind);
     }
   }
 }