Apply requested changes

Author: maikypedia

ruby/ql/lib/codeql/ruby/Frameworks.qll

@@ -34,4 +34,4 @@ private import codeql.ruby.frameworks.Twirp
 private import codeql.ruby.frameworks.Sqlite3
 private import codeql.ruby.frameworks.Rexml
 private import codeql.ruby.frameworks.Nokogiri
-private import codeql.ruby.frameworks.Libxml
+private import codeql.ruby.frameworks.LibXml

ruby/ql/lib/codeql/ruby/frameworks/Libxml.qll

@@ -9,12 +9,12 @@ private import codeql.ruby.Concepts
 /**
  * Provides modeling for `libxml`, an XML library for Ruby.
  */
-module Libxml {
+module LibXml {
   /**
    * Flow summary for `libxml`. Wraps a string, parsing it as an XML document.
    */
-  private class XMLSummary extends SummarizedCallable {
-    XMLSummary() { this = "LibXML::XML" }
+  private class XmlSummary extends SummarizedCallable {
+    XmlSummary() { this = "LibXML::XML" }
 
     override MethodCall getACall() { result = any(LibXmlRubyXmlParserCall c).asExpr().getExpr() }
 
@@ -24,15 +24,35 @@ module Libxml {
   }
 
   /** A call that parses XML. */
-  private class LibXmlRubyXmlParserCall extends DataFlow::CallNode {
-    LibXmlRubyXmlParserCall() {
+  abstract private class LibXmlRubyXmlParserCall extends XmlParserCall::Range, DataFlow::CallNode {
+  }
+
+  private class LibXmlRubyXmlParserCallString extends LibXmlRubyXmlParserCall {
+    LibXmlRubyXmlParserCallString() {
       this =
         [API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
             .getMember(["Document", "Parser"])
-            .getAMethodCall(["file", "io", "string"])
+            .getAMethodCall(["string"])
     }
 
-    DataFlow::Node getInput() { result = this.getArgument(0) }
+    override DataFlow::Node getInput() { result = this.getArgument(0) }
+
+    /** No option for parsing */
+    override predicate externalEntitiesEnabled() { none() }
+  }
+
+  private class LibXmlRubyXmlParserCallIoFile extends LibXmlRubyXmlParserCall {
+    LibXmlRubyXmlParserCallIoFile() {
+      this =
+        [API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
+            .getMember(["Document", "Parser"])
+            .getAMethodCall(["file", "io"])
+    }
+
+    override DataFlow::Node getInput() { result = this.getArgument(0) }
+
+    /** No option for parsing */
+    override predicate externalEntitiesEnabled() { none() }
   }
 
   /** Execution of a XPath statement. */

ruby/ql/lib/codeql/ruby/frameworks/Nokogiri.qll

@@ -13,8 +13,8 @@ module Nokogiri {
   /**
    * Flow summary for `nokogiri`. Wraps a string, parsing it as an XML document.
    */
-  private class XMLSummary extends SummarizedCallable {
-    XMLSummary() { this = "Nokogiri::XML.parse" }
+  private class XmlSummary extends SummarizedCallable {
+    XmlSummary() { this = "Nokogiri::XML.parse" }
 
     override MethodCall getACall() { result = any(NokogiriXmlParserCall p).asExpr().getExpr() }
 

ruby/ql/lib/codeql/ruby/frameworks/Rexml.qll

@@ -13,8 +13,8 @@ module Rexml {
   /**
    * Flow summary for `REXML::Document.new()`. This method wraps a string, parsing it as an XML document.
    */
-  private class XMLSummary extends SummarizedCallable {
-    XMLSummary() { this = "REXML::Document.new()" }
+  private class XmlSummary extends SummarizedCallable {
+    XmlSummary() { this = "REXML::Document.new()" }
 
     override MethodCall getACall() { result = any(RexmlParserCall c).asExpr().getExpr() }
 

ruby/ql/lib/codeql/ruby/security/XpathInjectionQuery.qll

@@ -10,15 +10,18 @@ private import codeql.ruby.DataFlow
 private import codeql.ruby.TaintTracking
 import XpathInjectionCustomizations::XpathInjection
 
-/**
- * A taint-tracking configuration for detecting "Xpath Injection" vulnerabilities.
- */
-class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "Xpath Injection" }
+/** Provides a taint-tracking configuration for detecting "Xpath Injection" vulnerabilities. */
+module XPathInjection {
+  /**
+   * A taint-tracking configuration for detecting "Xpath Injection" vulnerabilities.
+   */
+  private module Config implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { source instanceof Source }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+    predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 
-  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+    predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+  }
 
-  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+  import TaintTracking::Make<Config>
 }

ruby/ql/src/experimental/xpath-injection/XpathInjection.ql

@@ -1,7 +1,7 @@
 /**
  * @name XPath query built from user-controlled sources
  * @description Building a XPath query from user-controlled sources is vulnerable to insertion of
- *              malicious Xpath code by the user.
+ *              malicious XPath code by the user.
  * @kind path-problem
  * @problem.severity error
  * @security-severity 9.8
@@ -13,9 +13,9 @@
 
 import codeql.ruby.DataFlow
 import codeql.ruby.security.XpathInjectionQuery
-import DataFlow::PathGraph
+import XPathInjection::PathGraph
 
-from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
+from XPathInjection::PathNode source, XPathInjection::PathNode sink
+where XPathInjection::hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "XPath expression depends on a $@.", source.getNode(),
-  "user-provided value"
+  "user-provided value"