Java: remove path-injection related models and tests for now

This commit is contained in:
Jami Cogswell
2023-11-20 18:10:07 -05:00
parent 35a083ae9e
commit 915e106ab3
6 changed files with 12 additions and 626 deletions

View File

@@ -22,96 +22,7 @@ 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, "request-forgery")
or
sinkNode(this, "get-resource")
or
exists(MethodCall 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()
)
}
}
/** A sink for methods that load Spring resources. */
private class SpringResourceSink extends UnsafeUrlForwardSink {
SpringResourceSink() {
exists(MethodCall ma |
ma.getMethod() instanceof GetResourceUtilsMethod and
ma.getArgument(0) = this.asExpr()
)
}
}
// TODO: look into `StaplerResponse.forward`, etc., and think about re-adding the MaD "request-forgery" sinks as a result
/** An argument to `new ModelAndView` or `ModelAndView.setViewName`. */
private class SpringModelAndViewSink extends UnsafeUrlForwardSink {
SpringModelAndViewSink() {

View File

@@ -1,3 +1,5 @@
/** Provides configurations to be used in queries related to unsafe URL forwarding. */
import java
import semmle.code.java.security.UnsafeUrlForward
import semmle.code.java.dataflow.FlowSources
@@ -5,9 +7,13 @@ import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.Jsf
import semmle.code.java.security.PathSanitizer
/**
* A taint-tracking configuration for untrusted user input in a URL forward or include.
*/
module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source instanceof ThreatModelFlowSource and
// TODO: move below logic to class in UnsafeUrlForward.qll?
not exists(MethodCall ma, Method m | ma.getMethod() = m |
(
m instanceof HttpServletRequestGetRequestUriMethod or
@@ -25,21 +31,11 @@ module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
node instanceof PathInjectionSanitizer
}
// TODO: check if the below is still needed after removing path-injection related sinks.
DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
predicate isAdditionalFlowStep(DataFlow::Node prev, DataFlow::Node succ) {
exists(MethodCall 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()
)
}
}
/**
* Taint-tracking flow for untrusted user input in a URL forward or include.
*/
module UnsafeUrlForwardFlow = TaintTracking::Global<UnsafeUrlForwardFlowConfig>;