Java: Promote XSLT Injection from experimental#6097
Java: Promote XSLT Injection from experimental#6097aschackmull merged 10 commits intogithub:mainfrom
Conversation
|
|
|
@github/docs-content-codeql please review the qhelp file. Even though changes aren't introduced in this PR, it wasn't reviewed when this query was merged to experimental. |
|
|
|
|
mchammer01
left a comment
There was a problem hiding this comment.
@atorralba - this LGTM ✨
I'm making a minor suggestion to the query description + I fixed a few minor things in the qlhep file (hope it' ok for me to directly commit to your branch as I don't appear to be able to make suggestions to that file).
| import java | ||
| import semmle.code.java.dataflow.FlowSources | ||
| import XsltInjectionLib | ||
| import semmle.code.java.security.XsltInjectionQuery |
There was a problem hiding this comment.
Comment for line 3 above (as I can't comment on the line itself):
Suggesting:
Performing an XSLT transformation with user-controlled stylesheets can lead to information disclosure or execution of arbitrary code.
There was a problem hiding this comment.
Don't worry about directly committing to the branch, It's totally fine :-)
Thank you for the review! I applied your suggestion regarding the query description.
|
LGTM. |
Made a few minor tweaks during editorial review
05d344d to
78c12dc
Compare
|
Rebased to fix conflicts. |
|
|
PR to promote the XSLT Injection query created in #3363
Changes
XsltInjectionLib.qllfile was renamed and refactored to use the CSV sink model.InlineExpectationsTest.Evaluation
CVE-2012-1592 is correctly detected after adding ad hoc additional taint steps for
javax.servlet.ServletConext.getResourceandjava.net.URL.openStream. Considering adding the later as a flow summary in a separate PR (because of the impact it'll probably have in several queries).