Java: Add XXE sinks#6564
Conversation
|
This PR is currently a big rebase and merge mess -- recommend flattening unnecessary commits to make reviewing this easier. |
| * @kind path-problem | ||
| * @problem.severity error | ||
| * @precision high | ||
| * @id java/xxe |
There was a problem hiding this comment.
| * @id java/xxe | |
| * @id java/xxe-with-experimental-sinks |
| @@ -0,0 +1,31 @@ | |||
| /** | |||
| * @name Resolving XML external entity in user-controlled data | |||
There was a problem hiding this comment.
| * @name Resolving XML external entity in user-controlled data | |
| * @name Resolving XML external entity in user-controlled data (experimental sinks) |
| * @description Parsing user-controlled XML documents and allowing expansion of external entity | ||
| * references may lead to disclosure of confidential data or denial of service. |
There was a problem hiding this comment.
| * @description Parsing user-controlled XML documents and allowing expansion of external entity | |
| * references may lead to disclosure of confidential data or denial of service. | |
| * @description Parsing user-controlled XML documents and allowing expansion of external entity | |
| * references may lead to disclosure of confidential data or denial of service. | |
| * (note this version differs from query `java/xxe` by including support for additional possibly-vulnerable XML parsers) |
| } | ||
|
|
||
| /** | ||
| * The classes `org.apache.commons.digester3.Digester` or `org.apache.commons.digester.Digester` or `org.apache.tomcat.util.digester.Digester`. |
There was a problem hiding this comment.
| * The classes `org.apache.commons.digester3.Digester` or `org.apache.commons.digester.Digester` or `org.apache.tomcat.util.digester.Digester`. | |
| * The classes `org.apache.commons.digester3.Digester`, `org.apache.commons.digester.Digester` or `org.apache.tomcat.util.digester.Digester`. |
| @@ -0,0 +1,29 @@ | |||
| /** | |||
| * @name Resolving XML external entity in local source | |||
There was a problem hiding this comment.
Is there any reason to add this version? Did you need it to catch some CVE? If not, drop the local version.
There was a problem hiding this comment.
Yes i am using it to catch some cve.
| this.getMethod() = m and | ||
| m.getDeclaringType().hasQualifiedName("javax.xml.transform.sax", "SAXTransformerFactory") and | ||
| m.hasName("newXMLFilter") | ||
| m.getName() in ["newXMLFilter", "newTransformerHandler"] |
There was a problem hiding this comment.
Move this change to the experimental query
|
|
||
| /** An expression that always has the same string value. */ | ||
| private class ConstantStringExpr extends Expr { | ||
| class ConstantStringExpr extends Expr { |
There was a problem hiding this comment.
Copy this to your qll file rather than expose it: the heuristic is bad enough that we probably don't want to publicly expose this difficult-to-explain partial solution.
| } | ||
|
|
||
| /* | ||
| * SAXTransformer: https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#saxtransformerfactory |
There was a problem hiding this comment.
Undo this change: this file should be unchanged except exposing SafeTransformerFactoryFlowConfig which is acceptable, then the experimental query should add the extra sink.
…a version that uses local sources. Originally authored by @haby0, squashed to clean up a tangled commit history.
There are many XML parsers for Java, and most of them are vulnerable to XXE because their default settings enable parsing of
external entities. This query currently identifies vulnerable XML parsing from the following parsers: javax.xml.validation.Validator,
org.dom4j.DocumentHelper, org.apache.commons.digester3.Digester, java.beans.XMLDecoder, org.rundeck.api.parser.ParserHelper, org.apache.commons.digester.Digester, org.apache.tomcat.util.digester.Digester
Open source components: vertx-web