split localFieldStep into a load/store step for the DataFlow step

Author: erik-krogh

javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll

@@ -1591,20 +1591,41 @@ module DataFlow {
    */
   predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) {
     exists(ClassNode cls, string prop |
+      localFieldStoreStep(cls, pred, _, prop) and
+      localFieldLoadStep(cls, _, succ, prop)
+    )
+  }
+
+  private predicate localFieldStoreStep(
+    ClassNode cls, DataFlow::Node pred, DataFlow::Node succ, string prop
+  ) {
+    (
       pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyWrite(prop).getRhs()
       or
       // add support for writes on nested properties
       pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyRead(prop) and
       pred = any(DataFlow::PropRef ref).getBase()
       or
       pred = cls.getInstanceMethod(prop)
-    |
-      succ = cls.getAReceiverNode().getAPropertyRead(prop)
-    )
+    ) and
+    succ = cls.getConstructor().getReceiver()
+  }
+
+  private predicate localFieldLoadStep(
+    ClassNode cls, DataFlow::Node pred, DataFlow::Node succ, string prop
+  ) {
+    pred = cls.getConstructor().getReceiver() and
+    succ = cls.getAReceiverNode().getAPropertyRead(prop)
   }
 
   private class LocalFieldStep extends DataFlow::SharedFlowStep {
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { localFieldStep(pred, succ) }
+    override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+      localFieldLoadStep(_, pred, succ, prop)
+    }
+
+    override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
+      localFieldStoreStep(_, pred, succ, prop)
+    }
   }
 
   predicate argumentPassingStep = FlowSteps::argumentPassing/4;

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll

@@ -35,8 +35,4 @@ class Configration extends TaintTracking::Configuration {
     super.hasFlowPath(source, sink) and
     DataFlow::hasPathWithoutUnmatchedReturn(source, sink)
   }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-    DataFlow::localFieldStep(pred, succ)
-  }
 }

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginQuery.qll

@@ -26,8 +26,6 @@ class Configuration extends TaintTracking::Configuration {
   }
 
   override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
-    // jQuery plugins tend to be implemented as classes that store data in fields initialized by the constructor.
-    DataFlow::localFieldStep(src, sink) or
     aliasPropertyPresenceStep(src, sink)
   }
 

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionQuery.qll

@@ -32,8 +32,4 @@ class Configuration extends TaintTracking::Configuration {
     super.hasFlowPath(source, sink) and
     DataFlow::hasPathWithoutUnmatchedReturn(source, sink)
   }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-    DataFlow::localFieldStep(pred, succ)
-  }
 }

javascript/ql/src/Security/CWE-094/CodeInjection.ql

@@ -16,6 +16,7 @@
 import javascript
 import semmle.javascript.security.dataflow.CodeInjectionQuery
 import DataFlow::PathGraph
+import semmle.javascript.heuristics.AdditionalSources
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)