Python: Add support for detecting XSLT Injection (#3521 revived)#3801
Python: Add support for detecting XSLT Injection (#3521 revived)#3801tausbn merged 11 commits intogithub:masterfrom
Conversation
This PR adds support for detecting XSLT injection in Python. I have included the ql files as well as the tests with this.
As indicated here https://www.zaproxy.org/docs/alerts/90017/
yoff
left a comment
There was a problem hiding this comment.
In ParseXSLTArgument, the flow from tree.parse to tree could be generalised into a taint flow of its own. However, I would consider that out of scope for this PR.
python/ql/src/experimental/semmle/python/security/injection/XSLT.qll
Outdated
Show resolved
Hide resolved
python/ql/src/experimental/semmle/python/security/injection/XSLT.qll
Outdated
Show resolved
Hide resolved
tausbn
left a comment
There was a problem hiding this comment.
One minor documentation comment, otherwise LGTM.
| @@ -0,0 +1,35 @@ | |||
| /** | |||
| * @name Xslt query built from user-controlled sources | |||
There was a problem hiding this comment.
I know discussing how to write acronyms is all the rage, but perhaps this should be XSLT for consistency?
There was a problem hiding this comment.
In the name, totally agree! (fix implemented)
I think the discussion would decide whether it should be called XSLT.ql or Xslt.ql
|
@RasmusWL Thanks a lot for updating this and getting this merged into the code base. I just noticed that the ql used in #3801 #3521 #3522 won't detect the following case. I think this is a limitation in the current taint tracking library but I may be wrong. I am noting this down here to keep track of this. from lxml.etree import *
res = XPath('sink')or from lxml.etree import *
f = StringIO('<foo><bar></bar></foo>')
tree = parse(f)
r = tree.xpath("/tag[@id='%s']" % untrusted_value) |
This PR is bringing #3521 back from the dead (an external contribution).
I did some minor readjusting of the code, but nothing major.