Java: Refactor RequestForgery.ql

Author: aschackmull

java/ql/lib/semmle/code/java/security/RequestForgeryConfig.qll

@@ -8,9 +8,11 @@ import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.security.RequestForgery
 
 /**
+ * DEPRECATED: Use `RequestForgeryConfiguration` module instead.
+ *
  * A taint-tracking configuration characterising request-forgery risks.
  */
-class RequestForgeryConfiguration extends TaintTracking::Configuration {
+deprecated class RequestForgeryConfiguration extends TaintTracking::Configuration {
   RequestForgeryConfiguration() { this = "Server-Side Request Forgery" }
 
   override predicate isSource(DataFlow::Node source) {
@@ -29,3 +31,26 @@ class RequestForgeryConfiguration extends TaintTracking::Configuration {
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof RequestForgerySanitizer }
 }
+
+/**
+ * A taint-tracking configuration characterising request-forgery risks.
+ */
+module RequestForgeryConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source instanceof RemoteFlowSource and
+    // Exclude results of remote HTTP requests: fetching something else based on that result
+    // is no worse than following a redirect returned by the remote server, and typically
+    // we're requesting a resource via https which we trust to only send us to safe URLs.
+    not source.asExpr().(MethodAccess).getCallee() instanceof UrlConnectionGetInputStreamMethod
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgerySink }
+
+  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+    any(RequestForgeryAdditionalTaintStep r).propagatesTaint(pred, succ)
+  }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof RequestForgerySanitizer }
+}
+
+module RequestForgeryFlow = TaintTracking::Make<RequestForgeryConfiguration>;

java/ql/src/Security/CWE/CWE-918/RequestForgery.ql

@@ -13,9 +13,9 @@
 
 import java
 import semmle.code.java.security.RequestForgeryConfig
-import DataFlow::PathGraph
+import RequestForgeryFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, RequestForgeryConfiguration conf
-where conf.hasFlowPath(source, sink)
+from RequestForgeryFlow::PathNode source, RequestForgeryFlow::PathNode sink
+where RequestForgeryFlow::hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "Potential server-side request forgery due to a $@.",
   source.getNode(), "user-provided value"