Skip to content

Commit 8f9cd16

Browse files
committed
Update
1 parent d2f07e4 commit 8f9cd16

File tree

23 files changed

+786
-423
lines changed

23 files changed

+786
-423
lines changed

python/ql/src/experimental/Security/CWE-611/XXE.py

Lines changed: 0 additions & 13 deletions
This file was deleted.

python/ql/src/experimental/Security/CWE-611/XXE.ql

Lines changed: 0 additions & 22 deletions
This file was deleted.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
<?xml version="1.0"?>
2+
<!DOCTYPE dt [
3+
<!ENTITY xxe SYSTEM "file:///etc/passwd">]>
4+
<test>&xxe;</test>
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
from flask import request, Flask
2+
import lxml.etree
3+
import xml.etree.ElementTree
4+
5+
app = Flask(__name__)
6+
7+
# BAD
8+
@app.route("/bad")
9+
def bad():
10+
xml_content = request.args['xml_content']
11+
12+
parser = lxml.etree.XMLParser()
13+
parsed_xml = xml.etree.ElementTree.fromstring(xml_content, parser=parser)
14+
15+
return parsed_xml.text
16+
17+
# GOOD
18+
@app.route("/good")
19+
def good():
20+
xml_content = request.args['xml_content']
21+
22+
parser = lxml.etree.XMLParser(resolve_entities=False)
23+
parsed_xml = xml.etree.ElementTree.fromstring(xml_content, parser=parser)
24+
25+
return parsed_xml.text

python/ql/src/experimental/Security/CWE-611/XXE.qhelp renamed to python/ql/src/experimental/Security/CWE-611/XmlInjection.qhelp

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,18 @@
55

66
<overview>
77
<p>
8-
Parsing untrusted XML files with a weakly configured XML parser may lead to an XML External Entity (XXE) attack.
8+
Parsing untrusted XML files with a weakly configured XML parser may lead to attacks such as XML External Entity (XXE),
9+
Billion Laughs, Quadratic Blowup and DTD retrieval.
910
This type of attack uses external entity references to access arbitrary files on a system, carry out denial of
1011
service, or server side request forgery. Even when the result of parsing is not returned to the user, out-of-band
1112
data retrieval techniques may allow attackers to steal sensitive data. Denial of services can also be carried out
1213
in this situation.
1314
</p>
14-
<p>
15-
Refer to the following links to check the details regarding how and which libraries are vulnerable:
16-
</p>
17-
18-
<ul>
19-
<li><a href="https://docs.python.org/3/library/xml.html#xml-vulnerabilities">Python 3</a>.</li>
20-
<li><a href="https://docs.python.org/2/library/xml.html#xml-vulnerabilities">Python 2</a>.</li>
21-
</ul>
22-
23-
<p>
24-
This query currently identifies vulnerable XML parsing from the following parsers:
25-
<code>xml.etree.ElementTree.XMLParser</code>, <code>lxml.etree.XMLParser</code>, <code>lxml.etree.get_default_parser</code>,
26-
<code>xml.sax.make_parser</code>.
27-
</p>
2815
</overview>
2916

3017
<recommendation>
3118
<p>
32-
Use <a href="https://docs.python.org/3/library/xml.html#the-defusedxml-package">defusedxml</a>, a Python package aimed
19+
Use <a href="https://pypi.org/project/defusedxml/">defusedxml</a>, a Python package aimed
3320
to prevent any potentially malicious operation.
3421
</p>
3522
</recommendation>
@@ -39,10 +26,17 @@ to prevent any potentially malicious operation.
3926
The following example calls <code>xml.etree.ElementTree.fromstring</code> using a parser (<code>lxml.etree.XMLParser</code>)
4027
that is not safely configured on untrusted data, and is therefore inherently unsafe.
4128
</p>
42-
<sample src="XXE.py"/>
29+
<sample src="XmlInjection.py"/>
30+
<p>
31+
Providing an input (<code>xml_content</code>) like the following XML content against /bad, the request response would contain the contents of
32+
<code>/etc/passwd</code>.
33+
</p>
34+
<sample src="XXE.xml"/>
4335
</example>
4436

4537
<references>
38+
<li>Python 3 <a href="https://docs.python.org/3/library/xml.html#xml-vulnerabilities">XML Vulnerabilities</a>.</li>
39+
<li>Python 2 <a href="https://docs.python.org/2/library/xml.html#xml-vulnerabilities">XML Vulnerabilities</a>.</li>
4640
<li>Python <a href="https://www.edureka.co/blog/python-xml-parser-tutorial/">XML Parsing</a>.</li>
4741
<li>OWASP vulnerability description: <a href="https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing">XML External Entity (XXE) Processing</a>.</li>
4842
<li>OWASP guidance on parsing xml files: <a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#python">XXE Prevention Cheat Sheet</a>.</li>
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name XML injection
3+
* @description User input should not be parsed without security options enabled.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @id py/xml-injection
7+
* @tags security
8+
* external/cwe/cwe-611
9+
* external/cwe/cwe-776
10+
* external/cwe/cwe-827
11+
*/
12+
13+
// determine precision above
14+
import python
15+
import experimental.semmle.python.security.dataflow.XmlInjection
16+
import DataFlow::PathGraph
17+
18+
from DataFlow::PathNode source, DataFlow::PathNode sink, string kind
19+
where XmlInjection::xmlInjectionVulnerable(source, sink, kind)
20+
select sink.getNode(), source, sink,
21+
"$@ XML input is constructed from a $@ and is vulnerable to " + kind + ".", sink.getNode(),
22+
"This", source.getNode(), "user-provided value"

python/ql/src/experimental/semmle/python/Concepts.qll

Lines changed: 49 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -44,94 +44,84 @@ class LogOutput extends DataFlow::Node {
4444
DataFlow::Node getAnInput() { result = range.getAnInput() }
4545
}
4646

47-
/** Provides classes for modeling XML parsing APIs. */
48-
module XMLParsing {
47+
module XML {
4948
/**
5049
* A data-flow node that collects functions parsing XML.
5150
*
5251
* Extend this class to model new APIs. If you want to refine existing API models,
5352
* extend `XMLParsing` instead.
5453
*/
55-
abstract class Range extends DataFlow::Node {
54+
class XMLParsing extends DataFlow::Node instanceof XMLParsing::Range {
5655
/**
5756
* Gets the argument containing the content to parse.
5857
*/
59-
abstract DataFlow::Node getAnInput();
58+
DataFlow::Node getAnInput() { result = super.getAnInput() }
6059

6160
/**
62-
* Holds if the parser may be parsing the input dangerously.
63-
*
64-
* Specifically, this predicate holds whether the XML parsing parses/extends external
65-
* entities in the parsed XML stream.
61+
* Holds if the parsing method or the parser holding it is vulnerable to `kind`.
6662
*/
67-
abstract predicate mayBeDangerous();
63+
predicate vulnerable(string kind) { super.vulnerable(kind) }
6864
}
69-
}
7065

71-
/**
72-
* A data-flow node that collects functions parsing XML.
73-
*
74-
* Extend this class to model new APIs. If you want to refine existing API models,
75-
* extend `XMLParsing` instead.
76-
*/
77-
class XMLParsing extends DataFlow::Node instanceof XMLParsing::Range {
78-
/**
79-
* Gets the argument containing the content to parse.
80-
*
81-
* Specifically, this predicate holds whether the XML parsing parses/extends external
82-
* entities in the parsed XML stream.
83-
*/
84-
DataFlow::Node getAnInput() { result = super.getAnInput() }
85-
86-
/**
87-
* Holds if the parser may be parsing the input dangerously.
88-
*/
89-
predicate mayBeDangerous() { super.mayBeDangerous() }
90-
}
66+
/** Provides classes for modeling XML parsing APIs. */
67+
module XMLParsing {
68+
/**
69+
* A data-flow node that collects functions parsing XML.
70+
*
71+
* Extend this class to model new APIs. If you want to refine existing API models,
72+
* extend `XMLParsing` instead.
73+
*/
74+
abstract class Range extends DataFlow::Node {
75+
/**
76+
* Gets the argument containing the content to parse.
77+
*/
78+
abstract DataFlow::Node getAnInput();
79+
80+
/**
81+
* Holds if the parsing method or the parser holding it is vulnerable to `kind`.
82+
*/
83+
abstract predicate vulnerable(string kind);
84+
}
85+
}
9186

92-
/** Provides classes for modeling XML parsers. */
93-
module XMLParser {
9487
/**
9588
* A data-flow node that collects XML parsers.
9689
*
9790
* Extend this class to model new APIs. If you want to refine existing API models,
9891
* extend `XMLParser` instead.
9992
*/
100-
abstract class Range extends DataFlow::Node {
93+
class XMLParser extends DataFlow::Node instanceof XMLParser::Range {
10194
/**
10295
* Gets the argument containing the content to parse.
10396
*/
104-
abstract DataFlow::Node getAnInput();
97+
DataFlow::Node getAnInput() { result = super.getAnInput() }
10598

10699
/**
107-
* Holds if the parser may be dangerously configured.
108-
*
109-
* Specifically, this predicate holds whether the XML parser parses/extends external
110-
* entities in the parsed XML stream.
100+
* Holds if the parser is vulnerable to `kind`.
111101
*/
112-
abstract predicate mayBeDangerous();
102+
predicate vulnerable(string kind) { super.vulnerable(kind) }
113103
}
114-
}
115-
116-
/**
117-
* A data-flow node that collects XML parsers.
118-
*
119-
* Extend this class to model new APIs. If you want to refine existing API models,
120-
* extend `XMLParser` instead.
121-
*/
122-
class XMLParser extends DataFlow::Node instanceof XMLParser::Range {
123-
/**
124-
* Gets the argument containing the content to parse.
125-
*/
126-
DataFlow::Node getAnInput() { result = super.getAnInput() }
127104

128-
/**
129-
* Holds if the parser may be dangerously configured.
130-
*
131-
* Specifically, this predicate holds whether the XML parser parses/extends external
132-
* entities in the parsed XML stream.
133-
*/
134-
predicate mayBeDangerous() { super.mayBeDangerous() }
105+
/** Provides classes for modeling XML parsers. */
106+
module XMLParser {
107+
/**
108+
* A data-flow node that collects XML parsers.
109+
*
110+
* Extend this class to model new APIs. If you want to refine existing API models,
111+
* extend `XMLParser` instead.
112+
*/
113+
abstract class Range extends DataFlow::Node {
114+
/**
115+
* Gets the argument containing the content to parse.
116+
*/
117+
abstract DataFlow::Node getAnInput();
118+
119+
/**
120+
* Holds if the parser is vulnerable to `kind`.
121+
*/
122+
abstract predicate vulnerable(string kind);
123+
}
124+
}
135125
}
136126

137127
/** Provides classes for modeling LDAP query execution-related APIs. */

python/ql/src/experimental/semmle/python/Frameworks.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
private import experimental.semmle.python.frameworks.Stdlib
6-
private import experimental.semmle.python.frameworks.XML
6+
private import experimental.semmle.python.frameworks.Xml
77
private import experimental.semmle.python.frameworks.Flask
88
private import experimental.semmle.python.frameworks.Django
99
private import experimental.semmle.python.frameworks.Werkzeug

0 commit comments

Comments
 (0)