Skip to content

Commit 071a77c

Browse files
maikypediamaikypedia
committed
Ruby : XPath Injection Query (CWE-643)
1 parent a0a8468 commit 071a77c

File tree

17 files changed

+716
-0
lines changed

17 files changed

+716
-0
lines changed

‎ruby/ql/lib/codeql/ruby/Concepts.qll

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,58 @@ module XmlParserCall {
840840
}
841841
}
842842

843+
/**
844+
* A data-flow node that constructs an XPath expression.
845+
*
846+
* If it is important that the XPath expression is indeed executed, then use `XPathExecution`.
847+
*
848+
* Extend this class to refine existing API models. If you want to model new APIs,
849+
* extend `XPathConstruction::Range` instead.
850+
*/
851+
class XPathConstruction extends DataFlow::Node instanceof XPathConstruction::Range {
852+
/** Gets the argument that specifies the XPath expressions to be constructed. */
853+
DataFlow::Node getXPath() { result = super.getXPath() }
854+
}
855+
856+
/** Provides a class for modeling new XPath construction APIs. */
857+
module XPathConstruction {
858+
/**
859+
* A data-flow node that constructs an XPath expression.
860+
*
861+
* Extend this class to model new APIs. If you want to refine existing API models,
862+
* extend `XPathConstruction` instead.
863+
*/
864+
abstract class Range extends DataFlow::Node {
865+
/** Gets the argument that specifies the XPath expressions to be constructed. */
866+
abstract DataFlow::Node getXPath();
867+
}
868+
}
869+
870+
/**
871+
* A data-flow node that executes an XPath expression.
872+
*
873+
* Extend this class to refine existing API models. If you want to model new APIs,
874+
* extend `XPathExecution::Range` instead.
875+
*/
876+
class XPathExecution extends DataFlow::Node instanceof XPathExecution::Range {
877+
/** Gets the argument that specifies the XPath expressions to be executed. */
878+
DataFlow::Node getXPath() { result = super.getXPath() }
879+
}
880+
881+
/** Provides a class for modeling new XPath execution APIs. */
882+
module XPathExecution {
883+
/**
884+
* A data-flow node that executes an XPath expression.
885+
*
886+
* Extend this class to model new APIs. If you want to refine existing API models,
887+
* extend `XPathExecution` instead.
888+
*/
889+
abstract class Range extends DataFlow::Node {
890+
/** Gets the argument that specifies the XPath expressions to be executed. */
891+
abstract DataFlow::Node getXPath();
892+
}
893+
}
894+
843895
/**
844896
* A data-flow node that may represent a database object in an ORM system.
845897
*

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,6 @@ private import codeql.ruby.frameworks.Slim
3232
private import codeql.ruby.frameworks.Sinatra
3333
private import codeql.ruby.frameworks.Twirp
3434
private import codeql.ruby.frameworks.Sqlite3
35+
private import codeql.ruby.frameworks.Rexml
36+
private import codeql.ruby.frameworks.Nokogiri
37+
private import codeql.ruby.frameworks.Libxml

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Provides modeling for `libxml`, an XML library for Ruby.
3+
*/
4+
5+
private import codeql.ruby.ApiGraphs
6+
private import codeql.ruby.dataflow.FlowSummary
7+
private import codeql.ruby.Concepts
8+
9+
/**
10+
* Provides modeling for `libxml`, an XML library for Ruby.
11+
*/
12+
module Libxml {
13+
/**
14+
* Flow summary for `libxml`. Wraps a string, parsing it as an XML document.
15+
*/
16+
private class XMLSummary extends SummarizedCallable {
17+
XMLSummary() { this = "LibXML::XML" }
18+
19+
override MethodCall getACall() { result = any(LibXmlRubyXmlParserCall c).asExpr().getExpr() }
20+
21+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
22+
input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
23+
}
24+
}
25+
26+
/** A call that parses XML. */
27+
private class LibXmlRubyXmlParserCall extends DataFlow::CallNode {
28+
LibXmlRubyXmlParserCall() {
29+
this =
30+
[API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
31+
.getMember(["Document", "Parser"])
32+
.getAMethodCall(["file", "io", "string"])
33+
}
34+
35+
DataFlow::Node getInput() { result = this.getArgument(0) }
36+
}
37+
38+
/** Execution of a XPath statement. */
39+
private class LibXmlXPathExecution extends XPathExecution::Range, DataFlow::CallNode {
40+
LibXmlXPathExecution() {
41+
exists(LibXmlRubyXmlParserCall parserCall |
42+
this = parserCall.getAMethodCall(["find", "find_first"])
43+
)
44+
}
45+
46+
override DataFlow::Node getXPath() { result = this.getArgument(0) }
47+
}
48+
}

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/**
2+
* Provides modeling for `nokogiri`, an XML library for Ruby.
3+
*/
4+
5+
private import codeql.ruby.ApiGraphs
6+
private import codeql.ruby.dataflow.FlowSummary
7+
private import codeql.ruby.Concepts
8+
9+
/**
10+
* Provides modeling for `nokogiri`, an XML library for Ruby.
11+
*/
12+
module Nokogiri {
13+
/**
14+
* Flow summary for `nokogiri`. Wraps a string, parsing it as an XML document.
15+
*/
16+
private class XMLSummary extends SummarizedCallable {
17+
XMLSummary() { this = "Nokogiri::XML.parse" }
18+
19+
override MethodCall getACall() { result = any(NokogiriXmlParserCall p).asExpr().getExpr() }
20+
21+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
22+
input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
23+
}
24+
}
25+
26+
/** A call that parses XML. */
27+
private class NokogiriXmlParserCall extends DataFlow::CallNode {
28+
NokogiriXmlParserCall() {
29+
this =
30+
[
31+
API::getTopLevelMember("Nokogiri").getMember("XML"),
32+
API::getTopLevelMember("Nokogiri").getMember("XML").getMember("Document"),
33+
API::getTopLevelMember("Nokogiri")
34+
.getMember("XML")
35+
.getMember("SAX")
36+
.getMember("Parser")
37+
.getInstance()
38+
].getAMethodCall("parse")
39+
}
40+
41+
DataFlow::Node getInput() { result = this.getArgument(0) }
42+
}
43+
44+
/** Execution of a XPath statement. */
45+
private class NokogiriXPathExecution extends XPathExecution::Range, DataFlow::CallNode {
46+
NokogiriXPathExecution() {
47+
exists(NokogiriXmlParserCall parserCall |
48+
this = parserCall.getAMethodCall(["xpath", "at_xpath", "search", "at"])
49+
)
50+
}
51+
52+
override DataFlow::Node getXPath() { result = this.getArgument(0) }
53+
}
54+
}

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Provides modeling for `rexml`, an XML toolkit for Ruby.
3+
*/
4+
5+
private import codeql.ruby.ApiGraphs
6+
private import codeql.ruby.dataflow.FlowSummary
7+
private import codeql.ruby.Concepts
8+
9+
/**
10+
* Provides modeling for `rexml`, an XML toolkit for Ruby.
11+
*/
12+
module Rexml {
13+
/**
14+
* Flow summary for `REXML::Document.new()`. This method wraps a string, parsing it as an XML document.
15+
*/
16+
private class XMLSummary extends SummarizedCallable {
17+
XMLSummary() { this = "REXML::Document.new()" }
18+
19+
override MethodCall getACall() { result = any(RexmlParserCall c).asExpr().getExpr() }
20+
21+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
22+
input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
23+
}
24+
}
25+
26+
/** A call to `REXML::Document.new`, considered as a XML parsing. */
27+
private class RexmlParserCall extends XmlParserCall::Range, DataFlow::CallNode {
28+
RexmlParserCall() {
29+
this = API::getTopLevelMember("REXML").getMember("Document").getAnInstantiation()
30+
}
31+
32+
override DataFlow::Node getInput() { result = this.getArgument(0) }
33+
34+
/** No option for parsing */
35+
override predicate externalEntitiesEnabled() { none() }
36+
}
37+
38+
/** Execution of a XPath statement. */
39+
private class RexmlXPathExecution extends XPathExecution::Range, DataFlow::CallNode {
40+
RexmlXPathExecution() {
41+
this =
42+
[API::getTopLevelMember("REXML").getMember("XPath"), API::getTopLevelMember("XPath")]
43+
.getAMethodCall(["each", "first", "match"])
44+
}
45+
46+
override DataFlow::Node getXPath() { result = this.getArgument(1) }
47+
}
48+
}

‎ruby/ql/lib/codeql/ruby/security/XpathInjectionCustomizations.qll

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/**
2+
* Provides class and predicates to track external data that
3+
* may represent malicious xpath query objects.
4+
*
5+
* This module is intended to be imported into a taint-tracking query.
6+
*/
7+
8+
private import codeql.ruby.Concepts
9+
private import codeql.ruby.DataFlow
10+
private import codeql.ruby.dataflow.BarrierGuards
11+
private import codeql.ruby.dataflow.RemoteFlowSources
12+
13+
/** Models Xpath Injection related classes and functions */
14+
module XpathInjection {
15+
/** A data flow source for "XPath injection" vulnerabilities. */
16+
abstract class Source extends DataFlow::Node { }
17+
18+
/** A data flow sink for "XPath injection" vulnerabilities */
19+
abstract class Sink extends DataFlow::Node { }
20+
21+
/** A sanitizer for "XPath injection" vulnerabilities. */
22+
abstract class Sanitizer extends DataFlow::Node { }
23+
24+
/**
25+
* A source of remote user input, considered as a flow source.
26+
*/
27+
private class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
28+
29+
/**
30+
* An execution of an XPath expression, considered as a sink.
31+
*/
32+
private class XPathExecutionAsSink extends Sink {
33+
XPathExecutionAsSink() { this = any(XPathExecution e).getXPath() }
34+
}
35+
36+
/**
37+
* A construction of an XPath expression, considered as a sink.
38+
*/
39+
private class XPathConstructionAsSink extends Sink {
40+
XPathConstructionAsSink() { this = any(XPathConstruction c).getXPath() }
41+
}
42+
43+
/**
44+
* A comparison with a constant string, considered as a sanitizer-guard.
45+
*/
46+
private class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
47+
48+
/**
49+
* An inclusion check against an array of constant strings, considered as a
50+
* sanitizer-guard.
51+
*/
52+
private class StringConstArrayInclusionCallAsSanitizer extends Sanitizer,
53+
StringConstArrayInclusionCallBarrier { }
54+
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Provides a taint-tracking configuration for detecting "Xpath Injection" vulnerabilities.
3+
*
4+
* Note, for performance reasons: only import this file if
5+
* `XpathInjection::Configuration` is needed, otherwise
6+
* `XpathInjectionCustomizations` should be imported instead.
7+
*/
8+
9+
private import codeql.ruby.DataFlow
10+
private import codeql.ruby.TaintTracking
11+
import XpathInjectionCustomizations::XpathInjection
12+
13+
/**
14+
* A taint-tracking configuration for detecting "Xpath Injection" vulnerabilities.
15+
*/
16+
class Configuration extends TaintTracking::Configuration {
17+
Configuration() { this = "Xpath Injection" }
18+
19+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
20+
21+
override predicate isSink(DataFlow::Node source) { source instanceof Sink }
22+
23+
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
24+
}

‎ruby/ql/src/change-notes/2023-05-11-xpath-injection-query.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new experimental query, `rb/xpath-injection`, to detect cases where user input may be embedded into a template's code in an unsafe manner.

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
If an XPath expression is built using string concatenation, and the components of the concatenation
7+
include user input, it makes it very easy for a user to create a malicious XPath expression.
8+
</p>
9+
</overview>
10+
11+
<recommendation>
12+
<p>
13+
If user input must be included in an XPath expression, either sanitize the data or use variable
14+
references to safely embed it without altering the structure of the expression.
15+
</p>
16+
</recommendation>
17+
18+
<example>
19+
<p>
20+
The following example uses the <code>nokogiri</code>, <code>rexml</code> and <code>libxml</code> XML parsers to parse a string <code>xml</code>.
21+
Then the xpath query is controlled by the user and hence leads to a vulnerability.
22+
</p>
23+
<sample src="examples/XPathBad.rb"/>
24+
25+
<p>
26+
To guard against XPath Injection attacks, the user input should be sanitized.
27+
</p>
28+
<sample src="examples/XPathGood.rb"/>
29+
</example>
30+
31+
<references>
32+
<li>
33+
OWASP:
34+
<a href="https://owasp.org/www-community/attacks/XPATH_Injection">XPath injection</a>.
35+
</li>
36+
</references>
37+
</qhelp>

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name XPath query built from user-controlled sources
3+
* @description Building a XPath query from user-controlled sources is vulnerable to insertion of
4+
* malicious Xpath code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.8
8+
* @precision high
9+
* @id rb/xpath-injection
10+
* @tags security
11+
* external/cwe/cwe-643
12+
*/
13+
14+
import codeql.ruby.DataFlow
15+
import codeql.ruby.security.XpathInjectionQuery
16+
import DataFlow::PathGraph
17+
18+
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
19+
where config.hasFlowPath(source, sink)
20+
select sink.getNode(), source, sink, "XPath expression depends on a $@.", source.getNode(),
21+
"user-provided value"

0 commit comments

Comments
 (0)