behaviour preserving refactorization into modules

Author: erik-krogh

javascript/ql/src/Security/CWE-073/TemplateObjectInjection.ql

@@ -12,36 +12,7 @@
 
 import javascript
 import DataFlow::PathGraph
-import semmle.javascript.security.TaintedObject
-
-class TemplateObjInjectionConfig extends TaintTracking::Configuration {
-  TemplateObjInjectionConfig() { this = "TemplateObjInjectionConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
-    TaintedObject::isSource(source, label)
-  }
-
-  override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
-    label = TaintedObject::label() and
-    exists(MethodCallExpr mc |
-      Express::isResponse(mc.getReceiver()) and
-      mc.getMethodName() = "render" and
-      sink.asExpr() = mc.getArgument(1)
-    )
-  }
-
-  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
-    guard instanceof TaintedObject::SanitizerGuard
-  }
-
-  override predicate isAdditionalFlowStep(
-    DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
-  ) {
-    TaintedObject::step(src, trg, inlbl, outlbl)
-  }
-}
+import semmle.javascript.security.dataflow.TemplateObjectInjection::TemplateObjectInjection
 
 from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)

javascript/ql/src/semmle/javascript/frameworks/Express.qll

@@ -738,16 +738,32 @@ module Express {
    * as the value of a template variable.
    */
   private class TemplateInput extends HTTP::ResponseBody {
-    RouteHandler rh;
+    TemplateObjectInput obj;
 
     TemplateInput() {
+      obj.getALocalSource().(DataFlow::ObjectLiteralNode).hasPropertyWrite(_, this.flow())
+    }
+
+    override RouteHandler getRouteHandler() { result = obj.getRouteHandler() }
+  }
+
+  /**
+   * An object passed to the `render` method of an HTTP response object.
+   */
+  class TemplateObjectInput extends DataFlow::Node {
+    RouteHandler rh;
+
+    TemplateObjectInput() {
       exists(DataFlow::MethodCallNode render |
         render.calls(rh.getAResponseExpr().flow(), "render") and
-        this = render.getOptionArgument(1, _).asExpr()
+        this = render.getArgument(1)
       )
     }
 
-    override RouteHandler getRouteHandler() { result = rh }
+    /**
+     * Gets the route handler that uses this object.
+     */
+    RouteHandler getRouteHandler() { result = rh }
   }
 
   /**

javascript/ql/src/semmle/javascript/security/dataflow/TemplateObjectInjection.qll

@@ -0,0 +1,45 @@
+/**
+ * Provides a taint-tracking configuration for reasoning about
+ * template object injection vulnerabilities.
+ *
+ * Note, for performance reasons: only import this file if
+ * `TemplateObjectInjection::Configuration` is needed, otherwise
+ * `TemplateObjectInjectionCustomizations` should be imported instead.
+ */
+
+import javascript
+
+/**
+ * Provides a taint tracking configuration for reasoning template object injection vulnerabilities.
+ */
+module TemplateObjectInjection {
+  import TemplateObjectInjectionCustomizations::TemplateObjectInjection
+  private import semmle.javascript.security.TaintedObject
+
+  /**
+   * A taint tracking configuration for reasoning about template object injection vulnerabilities.
+   */
+  class TemplateObjInjectionConfig extends TaintTracking::Configuration {
+    TemplateObjInjectionConfig() { this = "TemplateObjInjectionConfig" }
+
+    override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
+      source.(Source).getAFlowLabel() = label
+    }
+
+    override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
+      sink instanceof Sink and label = TaintedObject::label()
+    }
+
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+
+    override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
+      guard instanceof TaintedObject::SanitizerGuard
+    }
+
+    override predicate isAdditionalFlowStep(
+      DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
+    ) {
+      TaintedObject::step(src, trg, inlbl, outlbl)
+    }
+  }
+}

javascript/ql/src/semmle/javascript/security/dataflow/TemplateObjectInjectionCustomizations.qll

@@ -0,0 +1,48 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * template object injection vulnerabilities, as well as extension points for
+ * adding your own.
+ */
+
+import javascript
+private import semmle.javascript.security.TaintedObjectCustomizations
+
+/**
+ * Provides sources, sinks and sanitizers for reasoning about
+ * template object injection vulnerabilities.
+ */
+module TemplateObjectInjection {
+  /**
+   * A data flow source for template object injection vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node {
+    /** Gets a flow label to associate with this source. */
+    abstract DataFlow::FlowLabel getAFlowLabel();
+  }
+
+  /**
+   * A data flow sink for template object injection vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for template object injection vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  private class TaintedObjectSourceAsSource extends Source {
+    TaintedObjectSourceAsSource() { this instanceof TaintedObject::Source }
+
+    override DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() }
+  }
+
+  private class RemoteFlowSourceAsSource extends Source {
+    RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
+
+    override DataFlow::FlowLabel getAFlowLabel() { result.isTaint() }
+  }
+
+  private class TemplateSink extends Sink {
+    TemplateSink() { this.asExpr() instanceof Express::TemplateObjectInput }
+  }
+}

javascript/ql/test/query-tests/Security/CWE-073/TemplateObjectInjection.qlref

@@ -1 +1 @@
-experimental/Security/CWE-073/TemplateObjectInjection.ql
+Security/CWE-073/TemplateObjectInjection.ql