Upload main structure and initial tests

github · RasmusWL · Mar 14, 2022 · Jun 19, 2021 · Jun 22, 2021 · Jun 29, 2021
commit 78deec84fc8ebd17e4aed6d108455e9aa3c58fce
diff --git a/python/ql/src/experimental/Security/CWE-611/XXE.ql b/python/ql/src/experimental/Security/CWE-611/XXE.ql
@@ -0,0 +1,20 @@
+/**
+ * @name XML External Entity abuse
+ * @description User input should not be parsed by XML parsers without security options enabled.
+ * @kind path-problem
+ * @problem.severity error
+ * @id py/xxe
+ * @tags security
+ *       external/cwe/cwe-611
+ */
+
+// determine precision above
+import python
+import experimental.semmle.python.security.XXE
+import DataFlow::PathGraph
+
+from XXEFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "$@ XML input is constructed from a $@ and isn't secured against XML External Entities abuse",
+  sink.getNode(), "This", source.getNode(), "user-provided value"
diff --git a/python/ql/src/experimental/Security/CWE-611/unit_tests/XXE.qlref b/python/ql/src/experimental/Security/CWE-611/unit_tests/XXE.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-611/XXE.ql
diff --git a/python/ql/src/experimental/Security/CWE-611/unit_tests/general.py b/python/ql/src/experimental/Security/CWE-611/unit_tests/general.py
@@ -0,0 +1,63 @@
+from flask import request, Flask
+from io import StringIO
+import xml.etree, xml.etree.ElementTree
+import lxml.etree
+import xml.dom.minidom, xml.dom.pulldom
+import xmltodict
+
+'''
+XML Parsers:
+  xml.etree.ElementTree.XMLParser() - no options, vuln by default
+  lxml.etree.XMLParser() - no_network=True huge_tree=False resolve_entities=True
+  lxml.etree.get_default_parser() - no options, default above options
+  xml.sax.make_parser() - parser.setFeature(xml.sax.handler.feature_external_ges, True)
+
+XML Parsing:
+  string:
+    xml.etree.ElementTree.fromstring(list)
+    xml.etree.ElementTree.XML
+    lxml.etree.fromstring(list)
+    lxml.etree.XML
+    xmltodict.parse
+
+  file StringIO(), BytesIO(b):
+    xml.etree.ElementTree.parse
+    lxml.etree.parse
+    xml.dom.(mini|pull)dom.parse(String)
+'''
+
+@app.route("/XMLParser-Empty&xml.etree.ElementTree.fromstring")
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  parser = lxml.etree.XMLParser()
+  return xml.etree.ElementTree.fromstring(xml_content, parser=parser).text # 'root...'
+
+@app.route("/XMLParser-Empty&xml.etree.ElementTree.parse")#!
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  parser = lxml.etree.XMLParser()
+  return xml.etree.ElementTree.parse(StringIO(xml_content), parser=parser).getroot().text # 'jorgectf'
+
+@app.route("/XMLParser-Empty&lxml.etree.fromstring")
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  parser = lxml.etree.XMLParser()
+  return lxml.etree.fromstring(xml_content, parser=parser).text # 'jorgectf'
+
+@app.route("/XMLParser-Empty&xml.etree.parse")#!
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  parser = lxml.etree.XMLParser()
+  return lxml.etree.parse(StringIO(xml_content), parser=parser).getroot().text # 'jorgectf'
+
+@app.route("/xmltodict-disable_entities_False")
+def test2():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  return xmltodict.parse(xml_content, disable_entities=False)
+
+
diff --git a/python/ql/src/experimental/Security/CWE-611/unit_tests/xml_sax_make_parser.py b/python/ql/src/experimental/Security/CWE-611/unit_tests/xml_sax_make_parser.py
@@ -0,0 +1,66 @@
+from io import StringIO
+import xml.sax
+
+# https://docs.python.org/3/library/xml.sax.handler.html#xml.sax.handler.feature_external_ges
+
+class MainHandler(xml.sax.ContentHandler):
+  def __init__(self):
+    self._result = []
+
+  def characters(self, data):
+    self._result.append(data)
+
+  def parse(self, f):
+    xml.sax.parse(f, self)
+    return self._result
+
+# GOOD
+@app.route("/MainHandler")
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  return MainHandler().parse(StringIO(xml_content))
+
+@app.route("/xml.sax.make_parser()+MainHandler")
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  BadHandler = MainHandler()
+  parser = xml.sax.make_parser()
+  parser.setContentHandler(BadHandler)
+  parser.parse(StringIO(xml_content))
+  return BadHandler._result
+
+@app.route("/xml.sax.make_parser()+MainHandler-xml.sax.handler.feature_external_ges_False")
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  BadHandler = MainHandler()
+  parser = xml.sax.make_parser()
+  parser.setContentHandler(BadHandler)
+  parser.setFeature(xml.sax.handler.feature_external_ges, False)
+  parser.parse(StringIO(xml_content))
+  return BadHandler._result
+
+# BAD
+@app.route("/xml.sax.make_parser()+MainHandler-xml.sax.handler.feature_external_ges_True")
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  GoodHandler = MainHandler()
+  parser = xml.sax.make_parser()
+  parser.setContentHandler(GoodHandler)
+  parser.setFeature(xml.sax.handler.feature_external_ges, True)
+  parser.parse(StringIO(xml_content))
+  return GoodHandler._result
+
+@app.route("/xml.sax.make_parser()+xml.dom.minidom.parse-xml.sax.handler.feature_external_ges_True")
+def test1():
+  xml_content = request.args['xml_content'] # <?xml version="1.0"?><!DOCTYPE dt [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><test>&xxe;</test>
+
+  parser = xml.sax.make_parser()
+  parser.setFeature(xml.sax.handler.feature_external_ges, True)
+  return xml.dom.minidom.parse(StringIO(xml_content), parser=parser).documentElement.childNodes
+
+
+
@@ -13,3 +13,46 @@ private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
+
+/** 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 if the parser may be parsing the input dangerously.
+     */
+    abstract predicate mayBeDangerous();
+  }
+}
+
+/**
+ * A data-flow node that collects functions setting HTTP Headers' content.
+ *
+ * Extend this class to model new APIs. If you want to refine existing API models,
+ * extend `XMLParsing` instead.
+ */
+class XMLParsing extends DataFlow::Node {
+  XMLParsing::Range range;
+
+  XMLParsing() { this = range }
+
+  /**
+   * Gets the argument containing the content to parse.
+   */
+  DataFlow::Node getAnInput() { result = range.getAnInput() }
+
+  /**
+   * Holds if the parser may be parsing the input dangerously.
+   */
+  predicate mayBeDangerous() { range.mayBeDangerous() }
+}
@@ -3,3 +3,4 @@
  */
 
 private import experimental.semmle.python.frameworks.Stdlib
+private import experimental.semmle.python.frameworks.XML
diff --git a/python/ql/src/experimental/semmle/python/security/XXE.qll b/python/ql/src/experimental/semmle/python/security/XXE.qll
@@ -0,0 +1,26 @@
+import python
+import experimental.semmle.python.Concepts
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.dataflow.new.TaintTracking
+import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.dataflow.new.BarrierGuards
+
+/**
+ * A taint-tracking configuration for detecting XML External entities abuse.
+ *
+ * This configuration uses `RemoteFlowSource` as a source because there's no
+ * risk at parsing not user-supplied input without security options enabled.
+ */
+class XXEFlowConfig extends TaintTracking::Configuration {
+  XXEFlowConfig() { this = "XXEFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(XMLParsing xmlParsing | xmlParsing.mayBeDangerous() and sink = xmlParsing.getAnInput())
+  }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
+}
diff --git a/python/ql/src/semmle/python/frameworks/XML.qll b/python/ql/src/semmle/python/frameworks/XML.qll
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3,3 +3,4 @@
		*/

		private import experimental.semmle.python.frameworks.Stdlib
		private import experimental.semmle.python.frameworks.XML