Skip to content

Java: CWE-552 Add sources and sinks to to detect unsafe getResource calls in Java EE applications #8706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions java/ql/lib/semmle/code/java/frameworks/Servlets.qll
Original file line number Diff line number Diff line change
Expand Up @@ -377,3 +377,26 @@ class RequestDispatchMethod extends Method {
this.hasName(["forward", "include"])
}
}

/**
* The interface `javax.servlet.ServletContext`.
*/
class ServletContext extends RefType {
ServletContext() { this.hasQualifiedName("javax.servlet", "ServletContext") }
}

/** The `getResource` method of `ServletContext`. */
class GetServletResourceMethod extends Method {
GetServletResourceMethod() {
this.getDeclaringType() instanceof ServletContext and
this.hasName("getResource")
}
}

/** The `getResourceAsStream` method of `ServletContext`. */
class GetServletResourceAsStreamMethod extends Method {
GetServletResourceAsStreamMethod() {
this.getDeclaringType() instanceof ServletContext and
this.hasName("getResourceAsStream")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// BAD: no URI validation
URL url = request.getServletContext().getResource(requestUrl);
url = getClass().getResource(requestUrl);
InputStream in = url.openStream();

InputStream in = request.getServletContext().getResourceAsStream(requestPath);
in = getClass().getClassLoader().getResourceAsStream(requestPath);

// GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix:
// (alternatively use `Path.normalize` instead of checking for `..`)
if (!requestPath.contains("..") && requestPath.startsWith("/trusted")) {
InputStream in = request.getServletContext().getResourceAsStream(requestPath);
}

Path path = Paths.get(requestUrl).normalize().toRealPath();
if (path.startsWith("/trusted")) {
URL url = request.getServletContext().getResource(path.toString());
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ attacks. It also shows how to remedy the problem by validating the user input.

<sample src="UnsafeServletRequestDispatch.java" />

<p>The following examples show an HTTP request parameter or request path being used directly to
retrieve a resource of a Java EE application without validating the input, which allows sensitive
file exposure attacks. It also shows how to remedy the problem by validating the user input.
</p>

<sample src="UnsafeResourceGet.java" />

</example>
<references>
<li>File Disclosure:
Expand All @@ -47,5 +54,8 @@ attacks. It also shows how to remedy the problem by validating the user input.
<li>Micro Focus:
<a href="https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.file_disclosure_j2ee">File Disclosure: J2EE</a>
</li>
<li>CVE-2015-5174:
<a href="https://vuldb.com/?id.81084">Apache Tomcat 6.0/7.0/8.0/9.0 Servletcontext getResource/getResourceAsStream/getResourcePaths Path Traversal</a>
</li>
</references>
</qhelp>
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import java
import UnsafeUrlForward
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking
import experimental.semmle.code.java.frameworks.Jsf
import experimental.semmle.code.java.PathSanitizer
import DataFlow::PathGraph

Expand Down Expand Up @@ -43,6 +44,20 @@ class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
override DataFlow::FlowFeature getAFeature() {
result instanceof DataFlow::FeatureHasSourceCallContext
}

override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) {
exists(MethodAccess ma |
(
ma.getMethod() instanceof GetServletResourceMethod or
ma.getMethod() instanceof GetFacesResourceMethod or
ma.getMethod() instanceof GetClassResourceMethod or
ma.getMethod() instanceof GetClassLoaderResourceMethod or
ma.getMethod() instanceof GetWildflyResourceMethod
) and
ma.getArgument(0) = prev.asExpr() and
ma = succ.asExpr()
)
}
}

from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeUrlForwardFlowConfig conf
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import java
private import experimental.semmle.code.java.frameworks.Jsf
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.FlowSources
private import semmle.code.java.dataflow.StringPrefixes
private import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions

/** A sink for unsafe URL forward vulnerabilities. */
abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
Expand All @@ -19,6 +21,84 @@ private class RequestDispatcherSink extends UnsafeUrlForwardSink {
}
}

/** The `getResource` method of `Class`. */
class GetClassResourceMethod extends Method {
GetClassResourceMethod() {
this.getDeclaringType() instanceof TypeClass and
this.hasName("getResource")
}
}

/** The `getResourceAsStream` method of `Class`. */
class GetClassResourceAsStreamMethod extends Method {
GetClassResourceAsStreamMethod() {
this.getDeclaringType() instanceof TypeClass and
this.hasName("getResourceAsStream")
}
}

/** The `getResource` method of `ClassLoader`. */
class GetClassLoaderResourceMethod extends Method {
GetClassLoaderResourceMethod() {
this.getDeclaringType() instanceof ClassLoaderClass and
this.hasName("getResource")
}
}

/** The `getResourceAsStream` method of `ClassLoader`. */
class GetClassLoaderResourceAsStreamMethod extends Method {
GetClassLoaderResourceAsStreamMethod() {
this.getDeclaringType() instanceof ClassLoaderClass and
this.hasName("getResourceAsStream")
}
}

/** The JBoss class `FileResourceManager`. */
class FileResourceManager extends RefType {
FileResourceManager() {
this.hasQualifiedName("io.undertow.server.handlers.resource", "FileResourceManager")
}
}

/** The JBoss method `getResource` of `FileResourceManager`. */
class GetWildflyResourceMethod extends Method {
GetWildflyResourceMethod() {
this.getDeclaringType().getASupertype*() instanceof FileResourceManager and
this.hasName("getResource")
}
}

/** The JBoss class `VirtualFile`. */
class VirtualFile extends RefType {
VirtualFile() { this.hasQualifiedName("org.jboss.vfs", "VirtualFile") }
}

/** The JBoss method `getChild` of `FileResourceManager`. */
class GetVirtualFileChildMethod extends Method {
GetVirtualFileChildMethod() {
this.getDeclaringType().getASupertype*() instanceof VirtualFile and
this.hasName("getChild")
}
}

/** An argument to `getResource()` or `getResourceAsStream()`. */
private class GetResourceSink extends UnsafeUrlForwardSink {
GetResourceSink() {
sinkNode(this, "open-url")
or
exists(MethodAccess ma |
(
ma.getMethod() instanceof GetServletResourceAsStreamMethod or
ma.getMethod() instanceof GetFacesResourceAsStreamMethod or
ma.getMethod() instanceof GetClassResourceAsStreamMethod or
ma.getMethod() instanceof GetClassLoaderResourceAsStreamMethod or
ma.getMethod() instanceof GetVirtualFileChildMethod
) and
ma.getArgument(0) = this.asExpr()
)
}
}

/** An argument to `new ModelAndView` or `ModelAndView.setViewName`. */
private class SpringModelAndViewSink extends UnsafeUrlForwardSink {
SpringModelAndViewSink() {
Expand Down Expand Up @@ -80,15 +160,18 @@ private class ServletGetPathSource extends SourceModelCsv {
}
}

/** Taint model related to `java.nio.file.Path`. */
/** Taint model related to `java.nio.file.Path` and `io.undertow.server.handlers.resource.Resource`. */
private class FilePathFlowStep extends SummaryModelCsv {
override predicate row(string row) {
row =
[
"java.nio.file;Paths;true;get;;;Argument[0..1];ReturnValue;taint",
"java.nio.file;Path;true;resolve;;;Argument[-1..0];ReturnValue;taint",
"java.nio.file;Path;true;normalize;;;Argument[-1];ReturnValue;taint",
"java.nio.file;Path;true;toString;;;Argument[-1];ReturnValue;taint"
"java.nio.file;Path;true;toString;;;Argument[-1];ReturnValue;taint",
"io.undertow.server.handlers.resource;Resource;true;getFile;;;Argument[-1];ReturnValue;taint",
"io.undertow.server.handlers.resource;Resource;true;getFilePath;;;Argument[-1];ReturnValue;taint",
"io.undertow.server.handlers.resource;Resource;true;getPath;;;Argument[-1];ReturnValue;taint"
]
}
}
34 changes: 34 additions & 0 deletions java/ql/src/experimental/semmle/code/java/frameworks/Jsf.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Provides classes and predicates for working with the Java Server Faces (JSF).
*/

import java

/**
* The JSF class `ExternalContext` for processing HTTP requests.
*/
class ExternalContext extends RefType {
ExternalContext() {
this.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "ExternalContext")
}
}

/**
* The method `getResource()` declared in JSF `ExternalContext`.
*/
class GetFacesResourceMethod extends Method {
GetFacesResourceMethod() {
this.getDeclaringType().getASupertype*() instanceof ExternalContext and
this.hasName("getResource")
}
}

/**
* The method `getResourceAsStream()` declared in JSF `ExternalContext`.
*/
class GetFacesResourceAsStreamMethod extends Method {
GetFacesResourceAsStreamMethod() {
this.getDeclaringType().getASupertype*() instanceof ExternalContext and
this.hasName("getResourceAsStream")
}
}
Loading