second round of UnsafeJQueryPlugin reuse

Author: erik-krogh

javascript/ql/src/Security/CWE-079/UnsafeJQueryPlugin.ql

@@ -16,7 +16,7 @@ import semmle.javascript.security.dataflow.UnsafeJQueryPlugin::UnsafeJQueryPlugi
 import DataFlow::PathGraph
 
 from
-  Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, JQueryPluginMethod plugin
+  Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, JQuery::JQueryPluginMethod plugin
 where
   cfg.hasFlowPath(source, sink) and
   source.getNode().(Source).getPlugin() = plugin and

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

@@ -497,24 +497,12 @@ module JQuery {
       }
     }
 
-    /**
-     * Gets a node that is backtracked from a node written to `$.fn[something]`.
-     */
-    private DataFlow::SourceNode writtenToJqueryFN(DataFlow::TypeBackTracker t) {
-      t.start() and result = any(DataFlow::Node plugin | jQueryPluginDefinition(_, plugin)).getALocalSource()
-      or
-      exists(DataFlow::TypeBackTracker t2 | result = writtenToJqueryFN(t2).backtrack(t2, t))
-    }
-
     /**
      * A `this` node in a JQuery plugin function, which is a JQuery object.
      */
     private class JQueryPluginThisObject extends Range {
       JQueryPluginThisObject() {
-        this =
-          DataFlow::thisNode(writtenToJqueryFN(DataFlow::TypeBackTracker::end())
-                .(DataFlow::FunctionNode)
-                .getFunction())
+        this = DataFlow::thisNode(any(JQueryPluginMethod method).getFunction())
       }
     }
   }
@@ -615,7 +603,7 @@ module JQuery {
   /**
    * Holds for jQuery plugin definitions of the form `$.fn.<pluginName> = <plugin>` or $.extend($.fn, {<pluginName>, <plugin>})`.
    */
-  predicate jQueryPluginDefinition(string pluginName, DataFlow::Node plugin) {
+  private predicate jQueryPluginDefinition(string pluginName, DataFlow::Node plugin) {
     exists(DataFlow::PropRead fn, DataFlow::PropWrite write |
       fn = jquery().getAPropertyRead("fn") and
       (
@@ -634,4 +622,41 @@ module JQuery {
       )
     )
   }
+
+  /**
+   * Gets a node that is registered as a jQuery plugin method at `def`.
+   */
+  private DataFlow::SourceNode getAJQueryPluginMethod(
+    DataFlow::TypeBackTracker t, DataFlow::Node def
+  ) {
+    t.start() and jQueryPluginDefinition(_, def) and result.flowsTo(def)
+    or
+    exists(DataFlow::TypeBackTracker t2 | result = getAJQueryPluginMethod(t2, def).backtrack(t2, t))
+  }
+
+  /**
+   * Gets a function that is registered as a jQuery plugin method at `def`.
+   */
+  private DataFlow::FunctionNode getAJQueryPluginMethod(DataFlow::Node def) {
+    result = getAJQueryPluginMethod(DataFlow::TypeBackTracker::end(), def)
+  }
+
+  /**
+   * A function that is registered as a jQuery plugin method.
+   */
+  class JQueryPluginMethod extends DataFlow::FunctionNode {
+    string pluginName;
+
+    JQueryPluginMethod() {
+      exists(DataFlow::Node def |
+        jQueryPluginDefinition(pluginName, def) and
+        this = getAJQueryPluginMethod(def)
+      )
+    }
+
+    /**
+     * Gets the name of this plugin.
+     */
+    string getPluginName() { result = pluginName }
+  }
 }

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

@@ -18,7 +18,7 @@ module UnsafeJQueryPlugin {
     /**
      * Gets the plugin that this source is used in.
      */
-    abstract JQueryPluginMethod getPlugin();
+    abstract JQuery::JQueryPluginMethod getPlugin();
   }
 
   /**
@@ -49,26 +49,6 @@ module UnsafeJQueryPlugin {
     }
   }
 
-  /**
-   * Gets a node that is registered as a jQuery plugin method at `def`.
-   */
-  private DataFlow::SourceNode getAJQueryPluginMethod(
-    DataFlow::TypeBackTracker t, DataFlow::Node def
-  ) {
-    t.start() and
-    jQueryPluginDefinition(_, def) and
-    result.flowsTo(def)
-    or
-    exists(DataFlow::TypeBackTracker t2 | result = getAJQueryPluginMethod(t2, def).backtrack(t2, t))
-  }
-
-  /**
-   * Gets a function that is registered as a jQuery plugin method at `def`.
-   */
-  private DataFlow::FunctionNode getAJQueryPluginMethod(DataFlow::Node def) {
-    result = getAJQueryPluginMethod(DataFlow::TypeBackTracker::end(), def)
-  }
-
   /**
    * Gets an operand to `extend`.
    */
@@ -86,29 +66,10 @@ module UnsafeJQueryPlugin {
     result = getAnExtendOperand(DataFlow::TypeBackTracker::end(), extend)
   }
 
-  /**
-   * A function that is registered as a jQuery plugin method.
-   */
-  class JQueryPluginMethod extends DataFlow::FunctionNode {
-    string pluginName;
-
-    JQueryPluginMethod() {
-      exists(DataFlow::Node def |
-        jQueryPluginDefinition(pluginName, def) and
-        this = getAJQueryPluginMethod(def)
-      )
-    }
-
-    /**
-     * Gets the name of this plugin.
-     */
-    string getPluginName() { result = pluginName }
-  }
-
   /**
    * Holds if `plugin` has a default option defined at `def`.
    */
-  private predicate hasDefaultOption(JQueryPluginMethod plugin, DataFlow::PropWrite def) {
+  private predicate hasDefaultOption(JQuery::JQueryPluginMethod plugin, DataFlow::PropWrite def) {
     exists(ExtendCall extend, JQueryPluginOptions options, DataFlow::SourceNode default |
       options.getPlugin() = plugin and
       options = getAnExtendOperand(extend) and
@@ -121,7 +82,7 @@ module UnsafeJQueryPlugin {
    * The client-provided options object for a jQuery plugin.
    */
   class JQueryPluginOptions extends DataFlow::ParameterNode {
-    JQueryPluginMethod method;
+    JQuery::JQueryPluginMethod method;
 
     JQueryPluginOptions() {
       exists(string optionsPattern |
@@ -142,7 +103,7 @@ module UnsafeJQueryPlugin {
     /**
      * Gets the plugin method that these options are used in.
      */
-    JQueryPluginMethod getPlugin() { result = method }
+    JQuery::JQueryPluginMethod getPlugin() { result = method }
   }
 
   /**
@@ -201,7 +162,9 @@ module UnsafeJQueryPlugin {
    * The client-provided options object for a jQuery plugin, considered as a source for unsafe jQuery plugins.
    */
   class JQueryPluginOptionsAsSource extends Source, JQueryPluginOptions {
-    override JQueryPluginMethod getPlugin() { result = JQueryPluginOptions.super.getPlugin() }
+    override JQuery::JQueryPluginMethod getPlugin() {
+      result = JQueryPluginOptions.super.getPlugin()
+    }
   }
 
   /**
@@ -223,7 +186,7 @@ module UnsafeJQueryPlugin {
   /**
    * Holds if `plugin` likely expects `sink` to be treated as a HTML fragment.
    */
-  predicate isLikelyIntentionalHtmlSink(JQueryPluginMethod plugin, Sink sink) {
+  predicate isLikelyIntentionalHtmlSink(JQuery::JQueryPluginMethod plugin, Sink sink) {
     exists(DataFlow::PropWrite defaultDef, string default, DataFlow::PropRead finalRead |
       hasDefaultOption(plugin, defaultDef) and
       defaultDef.getPropertyName() = finalRead.getPropertyName() and