Python: Port old experimental points-to based queries #13990
Conversation
I kept all the modeling in _one_ file, since that makes it easy to work with such an external contribution... and I would certainly propose this file setup for the future 👍
|
QHelp previews: python/ql/src/experimental/Security/CWE-091/XsltInjection.qhelpXSLT query built from user-controlled sourcesProcessing an unvalidated XSL stylesheet can allow an attacker to change the structure and contents of the resultant XML, include arbitrary files from the file system, or execute arbitrary code. RecommendationThis vulnerability can be prevented by not allowing untrusted user input to be passed as an XSL stylesheet. If the application logic necessitates processing untrusted XSL stylesheets, the input should be properly filtered and sanitized before use. ExampleIn the example below, the XSL stylesheet is controlled by the user and hence leads to a vulnerability. from lxml import etree
from io import StringIO
from flask import Flask, request
app = Flask(__name__)
@app.route("/xslt")
def bad():
xsltQuery = request.args.get('xml', '')
xslt_root = etree.XML(xsltQuery)
f = StringIO('<foo><bar></bar></foo>')
tree = etree.parse(f)
result_tree = tree.xslt(xslt_root) # Not OK |
Besides the Cheetah tests, which were missing from the query tests.
There was a problem hiding this comment.
I think it is possible to delete python/ql/src/experimental/semmle/python/security/injection/XSLT.qll as part of this?
There is also a suggested rename, but I am not sure it is important.
It would have been nice if the first commit had been broken up so that files do not both move (get merged) and change. But in this case, through a setup with two windows, it was not too bad to compare bits of one file containing new modelling with deleted files containing old modelling...
| * "template injection" | ||
| * vulnerabilities, as well as extension points for adding your own. | ||
| */ | ||
| module SqlInjection { |
There was a problem hiding this comment.
Should this module be renamed?
| /* Sources */ | ||
| import semmle.python.web.HttpRequest | ||
| /* Sinks */ | ||
| import experimental.semmle.python.security.injection.XSLT |
There was a problem hiding this comment.
yes, and the entire file is deleted in the end 👍
| } | ||
| } | ||
|
|
||
| // ----------------------------------------------------------------------------- |
There was a problem hiding this comment.
From here seems to be converted modelling from experimental.semmle.python.security.injection.XSLT.
There was a problem hiding this comment.
yep 👍 I've removed the XSLT.qll file now 👍 (0dca8a5)
Since all of this was ported already
My bad 😐 I've fixed things up, so should be good to go now 👍 |
Due to change in path-graph, and including LHS of assignments
I did not spend enough time on these to fully promote them, just took the minimal effort to move them off points-to so we can properly deprecate/remove that in the future 👍