don't import AdditionalSinks, refactor sink out in new HeuristicSinks instead

Author: erik-krogh

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

@@ -14,7 +14,7 @@
 // TODO: Proper customizations module, Source class Sink class etc.
 import javascript
 import DataFlow::PathGraph
-private import semmle.javascript.heuristics.AdditionalSinks
+private import semmle.javascript.heuristics.HeuristicSinks
 private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
 
 /**
@@ -58,7 +58,7 @@ private StringOps::ConcatenationLeaf sink() {
 
 private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
   t.start() and
-  result instanceof CodeInjection::Sink and
+  (result instanceof CodeInjection::Sink or result instanceof HeuristicCodeInjectionSink) and
   not result instanceof StringOps::ConcatenationRoot // the heuristic CodeInjection sink looks for string-concats, we are not interrested in those here.
   or
   exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, endsInCodeInjectionSink(t2)))

javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll

@@ -17,32 +17,12 @@ private import semmle.javascript.security.dataflow.RegExpInjectionCustomizations
 private import semmle.javascript.security.dataflow.ClientSideUrlRedirectCustomizations
 private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomizations
 private import semmle.javascript.security.dataflow.InsecureRandomnessCustomizations
+private import HeuristicSinks as Sinks
 
-/**
- * A heuristic sink for data flow in a security query.
- */
-abstract class HeuristicSink extends DataFlow::Node { }
+private class HeuristicSink = Sinks::HeuristicSink;
 
-private class HeuristicCodeInjectionSink extends HeuristicSink, CodeInjection::Sink {
-  HeuristicCodeInjectionSink() {
-    isAssignedTo(this, "$where")
-    or
-    isAssignedToOrConcatenatedWith(this, "(?i)(command|cmd|exec|code|script|program)")
-    or
-    isArgTo(this, "(?i)(eval|run)") // "exec" clashes too often with `RegExp.prototype.exec`
-    or
-    exists(string srcPattern |
-      // function/lambda syntax anywhere
-      srcPattern = "(?s).*function\\s*\\(.*\\).*" or
-      srcPattern = "(?s).*(\\(.*\\)|[A-Za-z_]+)\\s?=>.*"
-    |
-      isConcatenatedWithString(this, srcPattern)
-    )
-    or
-    // dynamic property name
-    isConcatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*")
-  }
-}
+private class HeuristicCodeInjectionSink extends Sinks::HeuristicCodeInjectionSink,
+  CodeInjection::Sink { }
 
 private class HeuristicCommandInjectionSink extends HeuristicSink, CommandInjection::Sink {
   HeuristicCommandInjectionSink() {

javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll

@@ -0,0 +1,35 @@
+/**
+ * Provides heuristically recognized sinks for security queries.
+ */
+
+import javascript
+private import SyntacticHeuristics
+
+/**
+ * A heuristic sink for data flow in a security query.
+ */
+abstract class HeuristicSink extends DataFlow::Node { }
+
+/**
+ * A heuristically recognized sink for `js/code-injection` vulnerabilities.
+ */
+class HeuristicCodeInjectionSink extends HeuristicSink {
+  HeuristicCodeInjectionSink() {
+    isAssignedTo(this, "$where")
+    or
+    isAssignedToOrConcatenatedWith(this, "(?i)(command|cmd|exec|code|script|program)")
+    or
+    isArgTo(this, "(?i)(eval|run)") // "exec" clashes too often with `RegExp.prototype.exec`
+    or
+    exists(string srcPattern |
+      // function/lambda syntax anywhere
+      srcPattern = "(?s).*function\\s*\\(.*\\).*" or
+      srcPattern = "(?s).*(\\(.*\\)|[A-Za-z_]+)\\s?=>.*"
+    |
+      isConcatenatedWithString(this, srcPattern)
+    )
+    or
+    // dynamic property name
+    isConcatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*")
+  }
+}