hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-27 17:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #13166 from atorralba/atorralba/java/xpath-xxe-sink

Browse Source 
Java: Add `XPath.evaluate` as XXE sink
This commit is contained in:
Tony Torralba
2023-05-16 09:14:56 +02:00
committed by GitHub
parent 57858afbd9 7d79d87d48
commit ac1df4de91
4 changed files with 59 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
java/ql/lib/semmle/code/java/security/XmlParsers.qll
View File

@@ -655,6 +655,11 @@ class XmlReader extends RefType {
XmlReader() { this.hasQualifiedName("org.xml.sax", "XMLReader") }
}
/** The class `org.xml.sax.InputSource`. */
class InputSource extends Class {
InputSource() { this.hasQualifiedName("org.xml.sax", "InputSource") }
}
/** DEPRECATED: Alias for XmlReader */
deprecated class XMLReader = XmlReader;
@@ -1164,22 +1169,34 @@ class XmlUnmarshal extends XmlParserCall {
}
/* XPathExpression: https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#xpathexpression */
/** The class `javax.xml.xpath.XPathExpression`. */
class XPathExpression extends RefType {
/** The interface `javax.xml.xpath.XPathExpression`. */
class XPathExpression extends Interface {
XPathExpression() { this.hasQualifiedName("javax.xml.xpath", "XPathExpression") }
}
/** A call to `XPathExpression.evaluate`. */
/** The interface `java.xml.xpath.XPath`. */
class XPath extends Interface {
XPath() { this.hasQualifiedName("javax.xml.xpath", "XPath") }
}
/** A call to the method `evaluate` of the classes `XPathExpression` or `XPath`. */
class XPathEvaluate extends XmlParserCall {
Argument sink;
XPathEvaluate() {
exists(Method m |
this.getMethod() = m and
m.getDeclaringType() instanceof XPathExpression and
m.hasName("evaluate")
|
m.getDeclaringType().getASourceSupertype*() instanceof XPathExpression and
sink = this.getArgument(0)
or
m.getDeclaringType().getASourceSupertype*() instanceof XPath and
sink = this.getArgument(1)
)
}
override Expr getSink() { result = this.getArgument(0) }
override Expr getSink() { result = sink }
override predicate isSafe() { none() }
}

4
java/ql/src/change-notes/2023-05-15-xpath-xxe-sink.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The queries `java/xxe` and `java/xxe-local` now recognize the second argument of calls to `XPath.evaluate` as a sink.

35
java/ql/test/query-tests/security/CWE-611/XPathExpressionTests.java
View File

@@ -12,18 +12,33 @@ public class XPathExpressionTests {
public void safeXPathExpression(Socket sock) throws Exception {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
DocumentBuilder builder = factory.newDocumentBuilder();
XPathFactory xFactory = XPathFactory.newInstance();
XPath path = xFactory.newXPath();
XPathExpression expr = path.compile("");
expr.evaluate(builder.parse(sock.getInputStream())); //safe
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
DocumentBuilder builder = factory.newDocumentBuilder();
XPathFactory xFactory = XPathFactory.newInstance();
XPath path = xFactory.newXPath();
XPathExpression expr = path.compile("");
expr.evaluate(builder.parse(sock.getInputStream())); // safe
}
public void unsafeExpressionTests(Socket sock) throws Exception {
XPathFactory xFactory = XPathFactory.newInstance();
XPath path = xFactory.newXPath();
XPathExpression expr = path.compile("");
expr.evaluate(new InputSource(sock.getInputStream())); //unsafe
XPathFactory xFactory = XPathFactory.newInstance();
XPath path = xFactory.newXPath();
XPathExpression expr = path.compile("");
expr.evaluate(new InputSource(sock.getInputStream())); // unsafe
}
public void safeXPathEvaluateTest(Socket sock) throws Exception {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
DocumentBuilder builder = factory.newDocumentBuilder();
XPathFactory xFactory = XPathFactory.newInstance();
XPath path = xFactory.newXPath();
path.evaluate("", builder.parse(sock.getInputStream()));
}
public void unsafeXPathEvaluateTest(Socket sock) throws Exception {
XPathFactory xFactory = XPathFactory.newInstance();
XPath path = xFactory.newXPath();
path.evaluate("", new InputSource(sock.getInputStream())); // unsafe
}
}

12
java/ql/test/query-tests/security/CWE-611/XXE.expected
View File

@@ -74,7 +74,8 @@ edges
| XMLReaderTests.java:86:34:86:54 | getInputStream(...) : InputStream | XMLReaderTests.java:86:18:86:55 | new InputSource(...) |
| XMLReaderTests.java:94:34:94:54 | getInputStream(...) : InputStream | XMLReaderTests.java:94:18:94:55 | new InputSource(...) |
| XMLReaderTests.java:100:34:100:54 | getInputStream(...) : InputStream | XMLReaderTests.java:100:18:100:55 | new InputSource(...) |
| XPathExpressionTests.java:27:37:27:57 | getInputStream(...) : InputStream | XPathExpressionTests.java:27:21:27:58 | new InputSource(...) |
| XPathExpressionTests.java:27:35:27:55 | getInputStream(...) : InputStream | XPathExpressionTests.java:27:19:27:56 | new InputSource(...) |
| XPathExpressionTests.java:42:39:42:59 | getInputStream(...) : InputStream | XPathExpressionTests.java:42:23:42:60 | new InputSource(...) |
nodes
| DocumentBuilderTests.java:14:19:14:39 | getInputStream(...) | semmle.label | getInputStream(...) |
| DocumentBuilderTests.java:28:19:28:39 | getInputStream(...) | semmle.label | getInputStream(...) |
@@ -235,8 +236,10 @@ nodes
| XMLReaderTests.java:94:34:94:54 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
| XMLReaderTests.java:100:18:100:55 | new InputSource(...) | semmle.label | new InputSource(...) |
| XMLReaderTests.java:100:34:100:54 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
| XPathExpressionTests.java:27:21:27:58 | new InputSource(...) | semmle.label | new InputSource(...) |
| XPathExpressionTests.java:27:37:27:57 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
| XPathExpressionTests.java:27:19:27:56 | new InputSource(...) | semmle.label | new InputSource(...) |
| XPathExpressionTests.java:27:35:27:55 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
| XPathExpressionTests.java:42:23:42:60 | new InputSource(...) | semmle.label | new InputSource(...) |
| XPathExpressionTests.java:42:39:42:59 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
| XmlInputFactoryTests.java:9:35:9:55 | getInputStream(...) | semmle.label | getInputStream(...) |
| XmlInputFactoryTests.java:10:34:10:54 | getInputStream(...) | semmle.label | getInputStream(...) |
| XmlInputFactoryTests.java:24:35:24:55 | getInputStream(...) | semmle.label | getInputStream(...) |
@@ -336,7 +339,8 @@ subpaths
| XMLReaderTests.java:86:18:86:55 | new InputSource(...) | XMLReaderTests.java:86:34:86:54 | getInputStream(...) : InputStream | XMLReaderTests.java:86:18:86:55 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XMLReaderTests.java:86:34:86:54 | getInputStream(...) | user-provided value |
| XMLReaderTests.java:94:18:94:55 | new InputSource(...) | XMLReaderTests.java:94:34:94:54 | getInputStream(...) : InputStream | XMLReaderTests.java:94:18:94:55 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XMLReaderTests.java:94:34:94:54 | getInputStream(...) | user-provided value |
| XMLReaderTests.java:100:18:100:55 | new InputSource(...) | XMLReaderTests.java:100:34:100:54 | getInputStream(...) : InputStream | XMLReaderTests.java:100:18:100:55 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XMLReaderTests.java:100:34:100:54 | getInputStream(...) | user-provided value |
| XPathExpressionTests.java:27:21:27:58 | new InputSource(...) | XPathExpressionTests.java:27:37:27:57 | getInputStream(...) : InputStream | XPathExpressionTests.java:27:21:27:58 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XPathExpressionTests.java:27:37:27:57 | getInputStream(...) | user-provided value |
| XPathExpressionTests.java:27:19:27:56 | new InputSource(...) | XPathExpressionTests.java:27:35:27:55 | getInputStream(...) : InputStream | XPathExpressionTests.java:27:19:27:56 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XPathExpressionTests.java:27:35:27:55 | getInputStream(...) | user-provided value |
| XPathExpressionTests.java:42:23:42:60 | new InputSource(...) | XPathExpressionTests.java:42:39:42:59 | getInputStream(...) : InputStream | XPathExpressionTests.java:42:23:42:60 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XPathExpressionTests.java:42:39:42:59 | getInputStream(...) | user-provided value |
| XmlInputFactoryTests.java:9:35:9:55 | getInputStream(...) | XmlInputFactoryTests.java:9:35:9:55 | getInputStream(...) | XmlInputFactoryTests.java:9:35:9:55 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XmlInputFactoryTests.java:9:35:9:55 | getInputStream(...) | user-provided value |
| XmlInputFactoryTests.java:10:34:10:54 | getInputStream(...) | XmlInputFactoryTests.java:10:34:10:54 | getInputStream(...) | XmlInputFactoryTests.java:10:34:10:54 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XmlInputFactoryTests.java:10:34:10:54 | getInputStream(...) | user-provided value |
| XmlInputFactoryTests.java:24:35:24:55 | getInputStream(...) | XmlInputFactoryTests.java:24:35:24:55 | getInputStream(...) | XmlInputFactoryTests.java:24:35:24:55 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XmlInputFactoryTests.java:24:35:24:55 | getInputStream(...) | user-provided value |