Skip to content

Commit 33ebcdf

Browse files
committed
Python: Support feed method of lxml/xml.etree Parsers
1 parent f72f673 commit 33ebcdf

File tree

3 files changed

+62
-0
lines changed

3 files changed

+62
-0
lines changed

python/ql/src/experimental/semmle/python/frameworks/Xml.qll

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,28 @@ private module Xml {
7979
}
8080
}
8181

82+
/**
83+
* A call to the `feed` method of an `xml.etree` parser.
84+
*/
85+
private class XMLEtreeParserFeedCall extends DataFlow::CallCfgNode, XML::XMLParsing::Range {
86+
XMLEtreeParserFeedCall() {
87+
this =
88+
API::moduleImport("xml")
89+
.getMember("etree")
90+
.getMember("ElementTree")
91+
.getMember("XMLParser")
92+
.getReturn()
93+
.getMember("feed")
94+
.getACall()
95+
}
96+
97+
override DataFlow::Node getAnInput() { result in [this.getArg(0), this.getArgByName("data")] }
98+
99+
override predicate vulnerable(XML::XMLVulnerabilityKind kind) {
100+
kind.isBillionLaughs() or kind.isQuadraticBlowup()
101+
}
102+
}
103+
82104
/**
83105
* A call to the `setFeature` method on a XML sax parser.
84106
*
@@ -322,6 +344,7 @@ private module Xml {
322344
}
323345

324346
override predicate vulnerable(XML::XMLVulnerabilityKind kind) {
347+
// TODO: This should be done with type-tracking
325348
exists(XML::XMLParser xmlParser |
326349
xmlParser = this.getArgByName("parser").getALocalSource() and xmlParser.vulnerable(kind)
327350
)
@@ -330,6 +353,33 @@ private module Xml {
330353
}
331354
}
332355

356+
/**
357+
* A call to the `feed` method of an `lxml.etree` parser.
358+
*/
359+
private class LXMLEtreeParserFeedCall extends DataFlow::MethodCallNode, XML::XMLParsing::Range {
360+
LXMLEtreeParserFeedCall() {
361+
exists(API::Node parserInstance |
362+
parserInstance =
363+
API::moduleImport("lxml").getMember("etree").getMember("XMLParser").getReturn()
364+
or
365+
parserInstance =
366+
API::moduleImport("lxml").getMember("etree").getMember("get_default_parser").getReturn()
367+
|
368+
this = parserInstance.getMember("feed").getACall()
369+
)
370+
}
371+
372+
override DataFlow::Node getAnInput() { result in [this.getArg(0), this.getArgByName("data")] }
373+
374+
override predicate vulnerable(XML::XMLVulnerabilityKind kind) {
375+
// TODO: This should be done with type-tracking
376+
exists(XML::XMLParser xmlParser |
377+
xmlParser = this.getObject().getALocalSource() and
378+
xmlParser.vulnerable(kind)
379+
)
380+
}
381+
}
382+
333383
/**
334384
* Gets a call to `xmltodict.parse`.
335385
*

python/ql/test/experimental/library-tests/frameworks/XML/lxml_etree.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@
2626
parser = lxml.etree.get_default_parser()
2727
lxml.etree.fromstring(x, parser=parser) # $ input=x vuln='XXE'
2828

29+
# manual use of feed method
30+
parser = lxml.etree.XMLParser()
31+
parser.feed(x) # $ input=x vuln='XXE'
32+
parser.feed(data=x) # $ input=x vuln='XXE'
33+
parser.close()
34+
2935
# XXE-safe
3036
parser = lxml.etree.XMLParser(resolve_entities=False)
3137
lxml.etree.fromstring(x, parser=parser) # $ input=x

python/ql/test/experimental/library-tests/frameworks/XML/xml_etree.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@
2727
parser = xml.etree.ElementTree.XMLParser()
2828
xml.etree.ElementTree.fromstring(x, parser=parser) # $ input=x vuln='Billion Laughs' vuln='Quadratic Blowup'
2929

30+
# manual use of feed method
31+
parser = xml.etree.ElementTree.XMLParser()
32+
parser.feed(x) # $ input=x vuln='Billion Laughs' vuln='Quadratic Blowup'
33+
parser.feed(data=x) # $ input=x vuln='Billion Laughs' vuln='Quadratic Blowup'
34+
parser.close()
35+
3036
# note: it's technically possible to use the thing wrapper func `fromstring` with an
3137
# `lxml` parser, and thereby change what vulnerabilities you are exposed to.. but it
3238
# seems very unlikely that anyone would do this, so we have intentionally not added any

0 commit comments

Comments
 (0)