XXE query

2026-04-25 08:45:14 +02:00 · 2021-09-02 16:34:01 +02:00
parent 0ea228e86f
commit 4268d9c565
11 changed files with 357 additions and 0 deletions
--- a/ql/lib/codeql/ruby/Concepts.qll
+++ b/ql/lib/codeql/ruby/Concepts.qll
@@ -503,3 +503,38 @@ module CodeExecution {
    abstract DataFlow::Node getCode();
  }
 }
+
+/**
+ * A data-flow node that parses XML content.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `XmlParserCall::Range` instead.
+ */
+class XmlParserCall extends DataFlow::Node {
+  XmlParserCall::Range range;
+
+  XmlParserCall() { this = range }
+
+  /** Gets the argument that specifies the XML content to be parsed. */
+  DataFlow::Node getInput() { result = range.getInput() }
+
+  /** Holds if this XML parser call is configured to process external entities */
+  predicate externalEntitiesEnabled() { range.externalEntitiesEnabled() }
+}
+
+/** Provides a class for modeling new XML parsing APIs. */
+module XmlParserCall {
+  /**
+   * A data-flow node that parses XML content.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `class XmlParserCall` instead.
+   */
+  abstract class Range extends DataFlow::Node {
+    /** Gets the argument that specifies the XML content to be parsed. */
+    abstract DataFlow::Node getInput();
+
+    /** Holds if this XML parser call is configured to process external entities */
+    abstract predicate externalEntitiesEnabled();
+  }
+}
--- a/ql/lib/codeql/ruby/Frameworks.qll
+++ b/ql/lib/codeql/ruby/Frameworks.qll
@@ -8,3 +8,4 @@ private import codeql.ruby.frameworks.ActionView
 private import codeql.ruby.frameworks.StandardLibrary
 private import codeql.ruby.frameworks.Files
 private import codeql.ruby.frameworks.HttpClients
+private import codeql.ruby.frameworks.XmlParsing
--- a/ql/lib/codeql/ruby/frameworks/XmlParsing.qll
+++ b/ql/lib/codeql/ruby/frameworks/XmlParsing.qll
@@ -0,0 +1,103 @@
+private import codeql.ruby.Concepts
+private import codeql.ruby.AST
+private import codeql.ruby.DataFlow
+private import codeql.ruby.typetracking.TypeTracker
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.controlflow.CfgNodes as CfgNodes
+
+private class NokogiriXmlParserCall extends XmlParserCall::Range, DataFlow::CallNode {
+  NokogiriXmlParserCall() {
+    this =
+      [
+        API::getTopLevelMember("Nokogiri").getMember("XML"),
+        API::getTopLevelMember("Nokogiri").getMember("XML").getMember("Document"),
+        API::getTopLevelMember("Nokogiri")
+            .getMember("XML")
+            .getMember("SAX")
+            .getMember("Parser")
+            .getInstance()
+      ].getAMethodCall("parse")
+  }
+
+  override DataFlow::Node getInput() { result = this.getArgument(0) }
+
+  override predicate externalEntitiesEnabled() {
+    this.getArgument(3) = trackNoEnt()
+    or
+    this.asExpr()
+        .getExpr()
+        .(MethodCall)
+        .getBlock()
+        .getAStmt()
+        .getAChild*()
+        .(MethodCall)
+        .getMethodName() = "noent"
+  }
+}
+
+private class LibXmlRubyXmlParserCall extends XmlParserCall::Range, DataFlow::CallNode {
+  LibXmlRubyXmlParserCall() {
+    this =
+      [API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
+          .getMember(["Document", "Parser"])
+          .getAMethodCall(["file", "io", "string"])
+  }
+
+  override DataFlow::Node getInput() { result = this.getArgument(0) }
+
+  override predicate externalEntitiesEnabled() {
+    exists(Pair pair |
+      pair = this.getArgument(1).asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+      pair.getKey().(Literal).getValueText() = "options" and
+      trackNoEnt().asExpr().getExpr() = pair.getValue()
+    )
+  }
+}
+
+private DataFlow::LocalSourceNode trackNoEnt(TypeTracker t) {
+  t.start() and
+  (
+    result.asExpr().getExpr().(IntegerLiteral).getValue().bitAnd(2) = 2
+    or
+    result =
+      API::getTopLevelMember("Nokogiri")
+          .getMember("XML")
+          .getMember("ParseOptions")
+          .getMember("NOENT")
+          .getAUse()
+    or
+    result =
+      [API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
+          .getMember("Options")
+          .getMember("NOENT")
+          .getAUse()
+    or
+    result.asExpr().getExpr() instanceof BitwiseOrExpr and
+    result.asExpr().(CfgNodes::ExprNodes::OperationCfgNode).getAnOperand() = trackNoEnt().asExpr()
+    or
+    result =
+      API::getTopLevelMember("Nokogiri")
+          .getMember("XML")
+          .getMember("ParseOptions")
+          .getAnInstantiation() and
+    result.asExpr().(CfgNodes::ExprNodes::CallCfgNode).getArgument(0) = trackNoEnt().asExpr()
+    or
+    exists(CfgNodes::ExprNodes::CallCfgNode call |
+      call.getExpr().(MethodCall).getMethodName() = "noent" and
+      (
+        result.asExpr() = call
+        or
+        result.flowsTo(any(DataFlow::Node n | n.asExpr() = call.getReceiver()))
+      )
+    )
+    or
+    exists(CfgNodes::ExprNodes::CallCfgNode call |
+      trackNoEnt().asExpr() = call.getReceiver() and
+      result.asExpr() = call
+    )
+  )
+  or
+  exists(TypeTracker t2 | result = trackNoEnt(t2).track(t2, t))
+}
+
+private DataFlow::Node trackNoEnt() { trackNoEnt(TypeTracker::end()).flowsTo(result) }
--- a/ql/src/queries/security/cwe-611/Xxe.qhelp
+++ b/ql/src/queries/security/cwe-611/Xxe.qhelp
@@ -0,0 +1,55 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Parsing untrusted XML files with a weakly configured XML parser may lead to an
+XML External Entity (XXE) attack. This type of attack uses external entity references
+to access arbitrary files on a system, carry out denial-of-service (DoS) attacks, or server-side
+request forgery. Even when the result of parsing is not returned to the user, DoS attacks are still possible
+and out-of-band data retrieval techniques may allow attackers to steal sensitive data.
+</p>
+</overview>
+
+<recommendation>
+<p>
+The easiest way to prevent XXE attacks is to disable external entity handling when
+parsing untrusted data. How this is done depends on the library being used. Note that some
+libraries, such as <code>rexml</code>, <code>nokogiri</code> and <code>libxml-ruby</code>, 
+disable entity expansion by default, so unless you have explicitly enabled entity expansion,
+no further action needs to be taken.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example uses the <code>nokogiri</code> XML parser to parse a string <code>xmlSrc</code>.
+If that string is from an untrusted source, this code may be vulnerable to an XXE attack, since
+the parser is invoked with the <code>noent</code> option set:
+</p>
+<sample src="examples/Xxe.rb"/>
+
+<p>
+To guard against XXE attacks, the <code>noent</code> option should be omitted or cleared
+(e.g. using <code>nonoent</code>). This means that no entity expansion is undertaken at all, 
+not even for standard internal entities such as <code>&amp;amp;</code> or <code>&amp;gt;</code>.
+If desired, these entities can be expanded in a separate step using utility functions.
+</p>
+<sample src="examples/XxeGood.rb"/>
+</example>
+
+<references>
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing">XML External Entity (XXE) Processing</a>.
+</li>
+<li>
+Timothy Morgen:
+<a href="https://research.nccgroup.com/2014/05/19/xml-schema-dtd-and-entity-attacks-a-compendium-of-known-techniques/">XML Schema, DTD, and Entity Attacks</a>.
+</li>
+<li>
+Timur Yunusov, Alexey Osipov:
+<a href="https://www.slideshare.net/qqlan/bh-ready-v4">XML Out-Of-Band Data Retrieval</a>.
+</li>
+</references>
+</qhelp>
--- a/ql/src/queries/security/cwe-611/Xxe.ql
+++ b/ql/src/queries/security/cwe-611/Xxe.ql
@@ -0,0 +1,43 @@
+/**
+ * @name XML external entity expansion
+ * @description Parsing user input as an XML document with external
+ *              entity expansion is vulnerable to XXE attacks.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 9.1
+ * @precision high
+ * @id rb/xxe
+ * @tags security
+ *       external/cwe/cwe-611
+ *       external/cwe/cwe-776
+ *       external/cwe/cwe-827
+ */
+
+import ruby
+import codeql.ruby.dataflow.RemoteFlowSources
+import codeql.ruby.TaintTracking
+import codeql.ruby.Concepts
+import codeql.ruby.DataFlow
+import DataFlow::PathGraph
+
+class UnsafeXxeSink extends DataFlow::ExprNode {
+  UnsafeXxeSink() {
+    exists(XmlParserCall parse |
+      parse.getInput() = this and
+      parse.externalEntitiesEnabled()
+    )
+  }
+}
+
+class XxeConfig extends TaintTracking::Configuration {
+  XxeConfig() { this = "XXE.ql::XxeConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(),
+  "user input"
--- a/ql/src/queries/security/cwe-611/examples/Xxe.rb
+++ b/ql/src/queries/security/cwe-611/examples/Xxe.rb
@@ -0,0 +1,12 @@
+require "nokogiri"
+
+def process_data1
+   xmlSrc = request.body
+   doc = Nokogiri::XML.parse(xmlSrc, nil, nil, Nokogiri::XML::ParseOptions::NOENT) # BAD
+end
+
+def process_data2
+   xmlSrc = request.body
+   doc = Nokogiri::XML.parse(xmlSrc) { |config| config.noent } # BAD
+end
+
--- a/ql/src/queries/security/cwe-611/examples/XxeGood.rb
+++ b/ql/src/queries/security/cwe-611/examples/XxeGood.rb
@@ -0,0 +1,12 @@
+require "nokogiri"
+
+def process_data1
+   xmlSrc = request.body
+   doc = Nokogiri::XML.parse(xmlSrc) # GOOD
+end
+
+def process_data2
+   xmlSrc = request.body
+   doc = Nokogiri::XML.parse(xmlSrc) { |config| config.nonoent } # GOOD
+end
+
--- a/ql/test/query-tests/security/cwe-611/LibXmlRuby.rb
+++ b/ql/test/query-tests/security/cwe-611/LibXmlRuby.rb
@@ -0,0 +1,16 @@
+class LibXmlRubyXXE < ApplicationController
+
+    content = params[:xml]
+    LibXML::XML::Document.string(content, { options: 2, encoding: 'utf-8' })
+    LibXML::XML::Document.file(content, { options: LibXML::XML::Options::NOENT })
+    LibXML::XML::Document.io(content, { options: XML::Options::NOENT })
+    LibXML::XML::Parser.string(content, { options: 2 })
+    LibXML::XML::Parser.file(content, { options: 3 })
+    LibXML::XML::Parser.io(content, { options: 2 })
+
+    XML::Document.string(content, { options: 2 })
+    XML::Parser.string(content, { options: 2 })
+
+    LibXML::XML::Parser.file(content, { options: 1 }) # OK
+
+end
--- a/ql/test/query-tests/security/cwe-611/Nokogiri.rb
+++ b/ql/test/query-tests/security/cwe-611/Nokogiri.rb
@@ -0,0 +1,22 @@
+class NokogiriXXE < ApplicationController
+
+    content = params[:xml]
+
+    Nokogiri::XML::parse(content, nil, nil, 2)
+    Nokogiri::XML::parse(content, nil, nil, 1 | 2)
+    Nokogiri::XML::parse(content, nil, nil, Nokogiri::XML::ParseOptions::NOENT)
+    Nokogiri::XML::parse(content, nil, nil, Nokogiri::XML::ParseOptions.new 2)
+    options = Nokogiri::XML::ParseOptions.new 0
+    options.noent
+    Nokogiri::XML::parse(content, nil, nil, options)
+    Nokogiri::XML::parse(content, nil, nil, (Nokogiri::XML::ParseOptions.new 0).noent)
+
+    Nokogiri::XML::parse(content) { |x| x.noent }
+
+    Nokogiri::XML::parse(content) { |x| x.nonet.noent.dtdload }
+
+    Nokogiri::XML::parse(content, nil, nil, 1) # OK
+    Nokogiri::XML::parse(content, nil, nil, 3)
+    Nokogiri::XML::parse(content) { |x| x.nonet.dtdload } # OK
+
+end
--- a/ql/test/query-tests/security/cwe-611/Xxe.expected
+++ b/ql/test/query-tests/security/cwe-611/Xxe.expected
@@ -0,0 +1,57 @@
+edges
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:4:34:4:40 | content |
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:5:32:5:38 | content |
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:6:30:6:36 | content |
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:7:32:7:38 | content |
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:8:30:8:36 | content |
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:9:28:9:34 | content |
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:11:26:11:32 | content |
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:12:24:12:30 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:5:26:5:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:6:26:6:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:7:26:7:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:8:26:8:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:11:26:11:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:12:26:12:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:14:26:14:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:16:26:16:32 | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:19:26:19:32 | content |
+nodes
+| LibXmlRuby.rb:3:15:3:20 | call to params :  | semmle.label | call to params :  |
+| LibXmlRuby.rb:4:34:4:40 | content | semmle.label | content |
+| LibXmlRuby.rb:5:32:5:38 | content | semmle.label | content |
+| LibXmlRuby.rb:6:30:6:36 | content | semmle.label | content |
+| LibXmlRuby.rb:7:32:7:38 | content | semmle.label | content |
+| LibXmlRuby.rb:8:30:8:36 | content | semmle.label | content |
+| LibXmlRuby.rb:9:28:9:34 | content | semmle.label | content |
+| LibXmlRuby.rb:11:26:11:32 | content | semmle.label | content |
+| LibXmlRuby.rb:12:24:12:30 | content | semmle.label | content |
+| Nokogiri.rb:3:15:3:20 | call to params :  | semmle.label | call to params :  |
+| Nokogiri.rb:5:26:5:32 | content | semmle.label | content |
+| Nokogiri.rb:6:26:6:32 | content | semmle.label | content |
+| Nokogiri.rb:7:26:7:32 | content | semmle.label | content |
+| Nokogiri.rb:8:26:8:32 | content | semmle.label | content |
+| Nokogiri.rb:11:26:11:32 | content | semmle.label | content |
+| Nokogiri.rb:12:26:12:32 | content | semmle.label | content |
+| Nokogiri.rb:14:26:14:32 | content | semmle.label | content |
+| Nokogiri.rb:16:26:16:32 | content | semmle.label | content |
+| Nokogiri.rb:19:26:19:32 | content | semmle.label | content |
+subpaths
+#select
+| LibXmlRuby.rb:4:34:4:40 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:4:34:4:40 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| LibXmlRuby.rb:5:32:5:38 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:5:32:5:38 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| LibXmlRuby.rb:6:30:6:36 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:6:30:6:36 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| LibXmlRuby.rb:7:32:7:38 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:7:32:7:38 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| LibXmlRuby.rb:8:30:8:36 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:8:30:8:36 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| LibXmlRuby.rb:9:28:9:34 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:9:28:9:34 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| LibXmlRuby.rb:11:26:11:32 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:11:26:11:32 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| LibXmlRuby.rb:12:24:12:30 | content | LibXmlRuby.rb:3:15:3:20 | call to params :  | LibXmlRuby.rb:12:24:12:30 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:5:26:5:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:5:26:5:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:6:26:6:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:6:26:6:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:7:26:7:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:7:26:7:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:8:26:8:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:8:26:8:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:11:26:11:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:11:26:11:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:12:26:12:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:12:26:12:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:14:26:14:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:14:26:14:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:16:26:16:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:16:26:16:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
+| Nokogiri.rb:19:26:19:32 | content | Nokogiri.rb:3:15:3:20 | call to params :  | Nokogiri.rb:19:26:19:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
--- a/ql/test/query-tests/security/cwe-611/Xxe.qlref
+++ b/ql/test/query-tests/security/cwe-611/Xxe.qlref
@@ -0,0 +1 @@
+queries/security/cwe-611/Xxe.ql