Java: Refactor StackTraceExposure, XSS.qll

Author: aschackmull

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

@@ -7,7 +7,7 @@ import semmle.code.java.frameworks.spring.SpringController
 import semmle.code.java.frameworks.spring.SpringHttp
 import semmle.code.java.frameworks.javaee.jsf.JSFRenderer
 import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.TaintTracking2
+import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.dataflow.ExternalFlow
 
 /** A sink that represent a method that outputs data without applying contextual output encoding. */
@@ -41,9 +41,9 @@ private class DefaultXssSink extends XssSink {
   DefaultXssSink() {
     sinkNode(this, "xss")
     or
-    exists(XssVulnerableWriterSourceToWritingMethodFlowConfig writer, MethodAccess ma |
+    exists(MethodAccess ma |
       ma.getMethod() instanceof WritingMethod and
-      writer.hasFlowToExpr(ma.getQualifier()) and
+      XssVulnerableWriterSourceToWritingMethodFlow::hasFlowToExpr(ma.getQualifier()) and
       this.asExpr() = ma.getArgument(_)
     )
   }
@@ -60,22 +60,19 @@ private class DefaultXssSanitizer extends XssSanitizer {
 }
 
 /** A configuration that tracks data from a servlet writer to an output method. */
-private class XssVulnerableWriterSourceToWritingMethodFlowConfig extends TaintTracking2::Configuration {
-  XssVulnerableWriterSourceToWritingMethodFlowConfig() {
-    this = "XSS::XssVulnerableWriterSourceToWritingMethodFlowConfig"
-  }
+private module XssVulnerableWriterSourceToWritingMethodFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof XssVulnerableWriterSource }
 
-  override predicate isSource(DataFlow::Node src) {
-    src.asExpr() instanceof XssVulnerableWriterSource
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(MethodAccess ma |
       sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod
     )
   }
 }
 
+private module XssVulnerableWriterSourceToWritingMethodFlow =
+  TaintTracking::Make<XssVulnerableWriterSourceToWritingMethodFlowConfig>;
+
 /** A method that can be used to output data to an output stream or writer. */
 private class WritingMethod extends Method {
   WritingMethod() {

java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql

@@ -31,33 +31,27 @@ class PrintStackTraceMethod extends Method {
   }
 }
 
-class ServletWriterSourceToPrintStackTraceMethodFlowConfig extends TaintTracking::Configuration {
-  ServletWriterSourceToPrintStackTraceMethodFlowConfig() {
-    this = "StackTraceExposure::ServletWriterSourceToPrintStackTraceMethodFlowConfig"
-  }
-
-  override predicate isSource(DataFlow::Node src) {
-    src.asExpr() instanceof XssVulnerableWriterSource
-  }
+module ServletWriterSourceToPrintStackTraceMethodFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof XssVulnerableWriterSource }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(MethodAccess ma |
       sink.asExpr() = ma.getAnArgument() and ma.getMethod() instanceof PrintStackTraceMethod
     )
   }
 }
 
+module ServletWriterSourceToPrintStackTraceMethodFlow =
+  TaintTracking::Make<ServletWriterSourceToPrintStackTraceMethodFlowConfig>;
+
 /**
  * A call that uses `Throwable.printStackTrace()` on a stream that is connected
  * to external output.
  */
 predicate printsStackToWriter(MethodAccess call) {
-  exists(
-    ServletWriterSourceToPrintStackTraceMethodFlowConfig writerSource,
-    PrintStackTraceMethod printStackTrace
-  |
+  exists(PrintStackTraceMethod printStackTrace |
     call.getMethod() = printStackTrace and
-    writerSource.hasFlowToExpr(call.getAnArgument())
+    ServletWriterSourceToPrintStackTraceMethodFlow::hasFlowToExpr(call.getAnArgument())
   )
 }
 
@@ -86,16 +80,15 @@ predicate stackTraceExpr(Expr exception, MethodAccess stackTraceString) {
   )
 }
 
-class StackTraceStringToHttpResponseSinkFlowConfig extends TaintTracking::Configuration {
-  StackTraceStringToHttpResponseSinkFlowConfig() {
-    this = "StackTraceExposure::StackTraceStringToHttpResponseSinkFlowConfig"
-  }
-
-  override predicate isSource(DataFlow::Node src) { stackTraceExpr(_, src.asExpr()) }
+module StackTraceStringToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { stackTraceExpr(_, src.asExpr()) }
 
-  override predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
 }
 
+module StackTraceStringToHttpResponseSinkFlow =
+  TaintTracking::Make<StackTraceStringToHttpResponseSinkFlowConfig>;
+
 /**
  * A write of stack trace data to an external stream.
  */
@@ -109,9 +102,10 @@ predicate printsStackExternally(MethodAccess call, Expr stackTrace) {
  * A stringified stack trace flows to an external sink.
  */
 predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stackTrace) {
-  exists(MethodAccess stackTraceString, StackTraceStringToHttpResponseSinkFlowConfig conf |
+  exists(MethodAccess stackTraceString |
     stackTraceExpr(stackTrace, stackTraceString) and
-    conf.hasFlow(DataFlow::exprNode(stackTraceString), externalExpr)
+    StackTraceStringToHttpResponseSinkFlow::hasFlow(DataFlow::exprNode(stackTraceString),
+      externalExpr)
   )
 }