Skip to content

Commit 4268d9c

Browse files
aibaarsaibaars
committed
XXE query
1 parent 0ea228e commit 4268d9c

File tree

11 files changed

+357
-0
lines changed

11 files changed

+357
-0
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,3 +503,38 @@ module CodeExecution {
503503
abstract DataFlow::Node getCode();
504504
}
505505
}
506+
507+
/**
508+
* A data-flow node that parses XML content.
509+
*
510+
* Extend this class to refine existing API models. If you want to model new APIs,
511+
* extend `XmlParserCall::Range` instead.
512+
*/
513+
class XmlParserCall extends DataFlow::Node {
514+
XmlParserCall::Range range;
515+
516+
XmlParserCall() { this = range }
517+
518+
/** Gets the argument that specifies the XML content to be parsed. */
519+
DataFlow::Node getInput() { result = range.getInput() }
520+
521+
/** Holds if this XML parser call is configured to process external entities */
522+
predicate externalEntitiesEnabled() { range.externalEntitiesEnabled() }
523+
}
524+
525+
/** Provides a class for modeling new XML parsing APIs. */
526+
module XmlParserCall {
527+
/**
528+
* A data-flow node that parses XML content.
529+
*
530+
* Extend this class to model new APIs. If you want to refine existing API models,
531+
* extend `class XmlParserCall` instead.
532+
*/
533+
abstract class Range extends DataFlow::Node {
534+
/** Gets the argument that specifies the XML content to be parsed. */
535+
abstract DataFlow::Node getInput();
536+
537+
/** Holds if this XML parser call is configured to process external entities */
538+
abstract predicate externalEntitiesEnabled();
539+
}
540+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ private import codeql.ruby.frameworks.ActionView
88
private import codeql.ruby.frameworks.StandardLibrary
99
private import codeql.ruby.frameworks.Files
1010
private import codeql.ruby.frameworks.HttpClients
11+
private import codeql.ruby.frameworks.XmlParsing

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
private import codeql.ruby.Concepts
2+
private import codeql.ruby.AST
3+
private import codeql.ruby.DataFlow
4+
private import codeql.ruby.typetracking.TypeTracker
5+
private import codeql.ruby.ApiGraphs
6+
private import codeql.ruby.controlflow.CfgNodes as CfgNodes
7+
8+
private class NokogiriXmlParserCall extends XmlParserCall::Range, DataFlow::CallNode {
9+
NokogiriXmlParserCall() {
10+
this =
11+
[
12+
API::getTopLevelMember("Nokogiri").getMember("XML"),
13+
API::getTopLevelMember("Nokogiri").getMember("XML").getMember("Document"),
14+
API::getTopLevelMember("Nokogiri")
15+
.getMember("XML")
16+
.getMember("SAX")
17+
.getMember("Parser")
18+
.getInstance()
19+
].getAMethodCall("parse")
20+
}
21+
22+
override DataFlow::Node getInput() { result = this.getArgument(0) }
23+
24+
override predicate externalEntitiesEnabled() {
25+
this.getArgument(3) = trackNoEnt()
26+
or
27+
this.asExpr()
28+
.getExpr()
29+
.(MethodCall)
30+
.getBlock()
31+
.getAStmt()
32+
.getAChild*()
33+
.(MethodCall)
34+
.getMethodName() = "noent"
35+
}
36+
}
37+
38+
private class LibXmlRubyXmlParserCall extends XmlParserCall::Range, DataFlow::CallNode {
39+
LibXmlRubyXmlParserCall() {
40+
this =
41+
[API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
42+
.getMember(["Document", "Parser"])
43+
.getAMethodCall(["file", "io", "string"])
44+
}
45+
46+
override DataFlow::Node getInput() { result = this.getArgument(0) }
47+
48+
override predicate externalEntitiesEnabled() {
49+
exists(Pair pair |
50+
pair = this.getArgument(1).asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
51+
pair.getKey().(Literal).getValueText() = "options" and
52+
trackNoEnt().asExpr().getExpr() = pair.getValue()
53+
)
54+
}
55+
}
56+
57+
private DataFlow::LocalSourceNode trackNoEnt(TypeTracker t) {
58+
t.start() and
59+
(
60+
result.asExpr().getExpr().(IntegerLiteral).getValue().bitAnd(2) = 2
61+
or
62+
result =
63+
API::getTopLevelMember("Nokogiri")
64+
.getMember("XML")
65+
.getMember("ParseOptions")
66+
.getMember("NOENT")
67+
.getAUse()
68+
or
69+
result =
70+
[API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
71+
.getMember("Options")
72+
.getMember("NOENT")
73+
.getAUse()
74+
or
75+
result.asExpr().getExpr() instanceof BitwiseOrExpr and
76+
result.asExpr().(CfgNodes::ExprNodes::OperationCfgNode).getAnOperand() = trackNoEnt().asExpr()
77+
or
78+
result =
79+
API::getTopLevelMember("Nokogiri")
80+
.getMember("XML")
81+
.getMember("ParseOptions")
82+
.getAnInstantiation() and
83+
result.asExpr().(CfgNodes::ExprNodes::CallCfgNode).getArgument(0) = trackNoEnt().asExpr()
84+
or
85+
exists(CfgNodes::ExprNodes::CallCfgNode call |
86+
call.getExpr().(MethodCall).getMethodName() = "noent" and
87+
(
88+
result.asExpr() = call
89+
or
90+
result.flowsTo(any(DataFlow::Node n | n.asExpr() = call.getReceiver()))
91+
)
92+
)
93+
or
94+
exists(CfgNodes::ExprNodes::CallCfgNode call |
95+
trackNoEnt().asExpr() = call.getReceiver() and
96+
result.asExpr() = call
97+
)
98+
)
99+
or
100+
exists(TypeTracker t2 | result = trackNoEnt(t2).track(t2, t))
101+
}
102+
103+
private DataFlow::Node trackNoEnt() { trackNoEnt(TypeTracker::end()).flowsTo(result) }

‎ql/src/queries/security/cwe-611/Xxe.qhelp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Parsing untrusted XML files with a weakly configured XML parser may lead to an
7+
XML External Entity (XXE) attack. This type of attack uses external entity references
8+
to access arbitrary files on a system, carry out denial-of-service (DoS) attacks, or server-side
9+
request forgery. Even when the result of parsing is not returned to the user, DoS attacks are still possible
10+
and out-of-band data retrieval techniques may allow attackers to steal sensitive data.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
The easiest way to prevent XXE attacks is to disable external entity handling when
17+
parsing untrusted data. How this is done depends on the library being used. Note that some
18+
libraries, such as <code>rexml</code>, <code>nokogiri</code> and <code>libxml-ruby</code>,
19+
disable entity expansion by default, so unless you have explicitly enabled entity expansion,
20+
no further action needs to be taken.
21+
</p>
22+
</recommendation>
23+
24+
<example>
25+
<p>
26+
The following example uses the <code>nokogiri</code> XML parser to parse a string <code>xmlSrc</code>.
27+
If that string is from an untrusted source, this code may be vulnerable to an XXE attack, since
28+
the parser is invoked with the <code>noent</code> option set:
29+
</p>
30+
<sample src="examples/Xxe.rb"/>
31+
32+
<p>
33+
To guard against XXE attacks, the <code>noent</code> option should be omitted or cleared
34+
(e.g. using <code>nonoent</code>). This means that no entity expansion is undertaken at all,
35+
not even for standard internal entities such as <code>&amp;amp;</code> or <code>&amp;gt;</code>.
36+
If desired, these entities can be expanded in a separate step using utility functions.
37+
</p>
38+
<sample src="examples/XxeGood.rb"/>
39+
</example>
40+
41+
<references>
42+
<li>
43+
OWASP:
44+
<a href="https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing">XML External Entity (XXE) Processing</a>.
45+
</li>
46+
<li>
47+
Timothy Morgen:
48+
<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>.
49+
</li>
50+
<li>
51+
Timur Yunusov, Alexey Osipov:
52+
<a href="https://www.slideshare.net/qqlan/bh-ready-v4">XML Out-Of-Band Data Retrieval</a>.
53+
</li>
54+
</references>
55+
</qhelp>

‎ql/src/queries/security/cwe-611/Xxe.ql

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @name XML external entity expansion
3+
* @description Parsing user input as an XML document with external
4+
* entity expansion is vulnerable to XXE attacks.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.1
8+
* @precision high
9+
* @id rb/xxe
10+
* @tags security
11+
* external/cwe/cwe-611
12+
* external/cwe/cwe-776
13+
* external/cwe/cwe-827
14+
*/
15+
16+
import ruby
17+
import codeql.ruby.dataflow.RemoteFlowSources
18+
import codeql.ruby.TaintTracking
19+
import codeql.ruby.Concepts
20+
import codeql.ruby.DataFlow
21+
import DataFlow::PathGraph
22+
23+
class UnsafeXxeSink extends DataFlow::ExprNode {
24+
UnsafeXxeSink() {
25+
exists(XmlParserCall parse |
26+
parse.getInput() = this and
27+
parse.externalEntitiesEnabled()
28+
)
29+
}
30+
}
31+
32+
class XxeConfig extends TaintTracking::Configuration {
33+
XxeConfig() { this = "XXE.ql::XxeConfig" }
34+
35+
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
36+
37+
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
38+
}
39+
40+
from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
41+
where conf.hasFlowPath(source, sink)
42+
select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(),
43+
"user input"

‎ql/src/queries/security/cwe-611/examples/Xxe.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
require "nokogiri"
2+
3+
def process_data1
4+
xmlSrc = request.body
5+
doc = Nokogiri::XML.parse(xmlSrc, nil, nil, Nokogiri::XML::ParseOptions::NOENT) # BAD
6+
end
7+
8+
def process_data2
9+
xmlSrc = request.body
10+
doc = Nokogiri::XML.parse(xmlSrc) { |config| config.noent } # BAD
11+
end
12+

‎ql/src/queries/security/cwe-611/examples/XxeGood.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
require "nokogiri"
2+
3+
def process_data1
4+
xmlSrc = request.body
5+
doc = Nokogiri::XML.parse(xmlSrc) # GOOD
6+
end
7+
8+
def process_data2
9+
xmlSrc = request.body
10+
doc = Nokogiri::XML.parse(xmlSrc) { |config| config.nonoent } # GOOD
11+
end
12+

‎ql/test/query-tests/security/cwe-611/LibXmlRuby.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
class LibXmlRubyXXE < ApplicationController
2+
3+
content = params[:xml]
4+
LibXML::XML::Document.string(content, { options: 2, encoding: 'utf-8' })
5+
LibXML::XML::Document.file(content, { options: LibXML::XML::Options::NOENT })
6+
LibXML::XML::Document.io(content, { options: XML::Options::NOENT })
7+
LibXML::XML::Parser.string(content, { options: 2 })
8+
LibXML::XML::Parser.file(content, { options: 3 })
9+
LibXML::XML::Parser.io(content, { options: 2 })
10+
11+
XML::Document.string(content, { options: 2 })
12+
XML::Parser.string(content, { options: 2 })
13+
14+
LibXML::XML::Parser.file(content, { options: 1 }) # OK
15+
16+
end

‎ql/test/query-tests/security/cwe-611/Nokogiri.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
class NokogiriXXE < ApplicationController
2+
3+
content = params[:xml]
4+
5+
Nokogiri::XML::parse(content, nil, nil, 2)
6+
Nokogiri::XML::parse(content, nil, nil, 1 | 2)
7+
Nokogiri::XML::parse(content, nil, nil, Nokogiri::XML::ParseOptions::NOENT)
8+
Nokogiri::XML::parse(content, nil, nil, Nokogiri::XML::ParseOptions.new 2)
9+
options = Nokogiri::XML::ParseOptions.new 0
10+
options.noent
11+
Nokogiri::XML::parse(content, nil, nil, options)
12+
Nokogiri::XML::parse(content, nil, nil, (Nokogiri::XML::ParseOptions.new 0).noent)
13+
14+
Nokogiri::XML::parse(content) { |x| x.noent }
15+
16+
Nokogiri::XML::parse(content) { |x| x.nonet.noent.dtdload }
17+
18+
Nokogiri::XML::parse(content, nil, nil, 1) # OK
19+
Nokogiri::XML::parse(content, nil, nil, 3)
20+
Nokogiri::XML::parse(content) { |x| x.nonet.dtdload } # OK
21+
22+
end

‎ql/test/query-tests/security/cwe-611/Xxe.expected

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
edges
2+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:4:34:4:40 | content |
3+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:5:32:5:38 | content |
4+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:6:30:6:36 | content |
5+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:7:32:7:38 | content |
6+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:8:30:8:36 | content |
7+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:9:28:9:34 | content |
8+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:11:26:11:32 | content |
9+
| LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:12:24:12:30 | content |
10+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:5:26:5:32 | content |
11+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:6:26:6:32 | content |
12+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:7:26:7:32 | content |
13+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:8:26:8:32 | content |
14+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:11:26:11:32 | content |
15+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:12:26:12:32 | content |
16+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:14:26:14:32 | content |
17+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:16:26:16:32 | content |
18+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:19:26:19:32 | content |
19+
nodes
20+
| LibXmlRuby.rb:3:15:3:20 | call to params : | semmle.label | call to params : |
21+
| LibXmlRuby.rb:4:34:4:40 | content | semmle.label | content |
22+
| LibXmlRuby.rb:5:32:5:38 | content | semmle.label | content |
23+
| LibXmlRuby.rb:6:30:6:36 | content | semmle.label | content |
24+
| LibXmlRuby.rb:7:32:7:38 | content | semmle.label | content |
25+
| LibXmlRuby.rb:8:30:8:36 | content | semmle.label | content |
26+
| LibXmlRuby.rb:9:28:9:34 | content | semmle.label | content |
27+
| LibXmlRuby.rb:11:26:11:32 | content | semmle.label | content |
28+
| LibXmlRuby.rb:12:24:12:30 | content | semmle.label | content |
29+
| Nokogiri.rb:3:15:3:20 | call to params : | semmle.label | call to params : |
30+
| Nokogiri.rb:5:26:5:32 | content | semmle.label | content |
31+
| Nokogiri.rb:6:26:6:32 | content | semmle.label | content |
32+
| Nokogiri.rb:7:26:7:32 | content | semmle.label | content |
33+
| Nokogiri.rb:8:26:8:32 | content | semmle.label | content |
34+
| Nokogiri.rb:11:26:11:32 | content | semmle.label | content |
35+
| Nokogiri.rb:12:26:12:32 | content | semmle.label | content |
36+
| Nokogiri.rb:14:26:14:32 | content | semmle.label | content |
37+
| Nokogiri.rb:16:26:16:32 | content | semmle.label | content |
38+
| Nokogiri.rb:19:26:19:32 | content | semmle.label | content |
39+
subpaths
40+
#select
41+
| 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 |
42+
| 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 |
43+
| 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 |
44+
| 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 |
45+
| 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 |
46+
| 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 |
47+
| 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 |
48+
| 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 |
49+
| 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 |
50+
| 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 |
51+
| 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 |
52+
| 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 |
53+
| 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 |
54+
| 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 |
55+
| 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 |
56+
| 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 |
57+
| 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 |

0 commit comments

Comments
 (0)