From 373a437d71a2d4dcde7627ec4b1500da958d6f8f Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Wed, 10 Jun 2020 19:40:26 +0200
Subject: [PATCH 1/6] add query to detect improperly sanitized code

---
 .../CWE-094/ImproperCodeSanitization.qhelp    | 37 ++++++++++
 .../CWE-094/ImproperCodeSanitization.ql       | 74 +++++++++++++++++++
 .../ImproperCodeSanitization.expected         | 45 +++++++++++
 .../ImproperCodeSanitization.qlref            |  1 +
 .../CodeInjection/bad-code-sanitization.js    | 45 +++++++++++
 5 files changed, 202 insertions(+)
 create mode 100644 javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp
 create mode 100644 javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
 create mode 100644 javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
 create mode 100644 javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.qlref
 create mode 100644 javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js

diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp
new file mode 100644
index 00000000000..16196386d12
--- /dev/null
+++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp
@@ -0,0 +1,37 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Placeholder
+</p>
+</overview>
+
+<recommendation>
+<p>
+Placeholder
+</p>
+</recommendation>
+
+<example>
+<p>
+Placeholder
+</p>
+
+</example>
+
+<references>
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
+</li>
+<li>
+MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects#Function_properties">Global functions</a>.
+</li>
+<li>
+MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function">Function constructor</a>.
+</li>
+</references>
+</qhelp>
diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
new file mode 100644
index 00000000000..87f97e8060b
--- /dev/null
+++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
@@ -0,0 +1,74 @@
+/**
+ * @name Improper code sanitization
+ * @description Escaping code as HTML does not provide protection against code-injection.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id js/bad-code-sanitization
+ * @tags security
+ *       external/cwe/cwe-094
+ *       external/cwe/cwe-079
+ *       external/cwe/cwe-116
+ */
+
+// TODO: Proper customizations module, Source class Sink class etc.
+import javascript
+import DataFlow::PathGraph
+private import semmle.javascript.heuristics.AdditionalSinks
+private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
+
+/**
+ * A taint-tracking configuration for reasoning about improper code sanitization vulnerabilities.
+ */
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "ImproperCodeSanitization" }
+
+  override predicate isSource(DataFlow::Node source) { source = source() }
+
+  override predicate isSink(DataFlow::Node sink) { sink = sink() }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof StringReplaceCall // any string-replace that happens after the bad-sanitizer, is assumed to be a good sanitizer.
+    // TODO: Specialize? This regexp sanitizes: /[<>\b\f\n\r\t\0\u2028\u2029]/g
+  }
+}
+
+private DataFlow::Node source() {
+  result instanceof HtmlSanitizerCall
+  or
+  result = DataFlow::globalVarRef("JSON").getAMemberCall("stringify")
+}
+
+private StringOps::ConcatenationLeaf sink() {
+  exists(StringOps::ConcatenationRoot root, int i |
+    root.getOperand(i) = result and
+    not exists(result.getStringValue()) and
+    not root = endsInCodeInjectionSink()
+  |
+    exists(StringOps::ConcatenationLeaf functionLeaf |
+      functionLeaf = root.getOperand(any(int j | j < i))
+    |
+      functionLeaf
+          .getStringValue()
+          .regexpMatch([".*function( )?([a-zA-Z0-9]+)?( )?\\(.*", ".*eval\\(.*",
+                ".*new Function\\(.*", "(^|.*[^a-zA-Z0-9])\\(.*\\)( )?=>.*"])
+    )
+  )
+}
+
+private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
+  t.start() and
+  result instanceof CodeInjection::Sink 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)))
+}
+
+private DataFlow::Node endsInCodeInjectionSink() {
+  result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end())
+}
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "$@ flows to here and is used to construct code.",
+  source.getNode(), "Improperly sanitized value"
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
new file mode 100644
index 00000000000..26c13d2140d
--- /dev/null
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
@@ -0,0 +1,45 @@
+nodes
+| bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` |
+| bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` |
+| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) |
+| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) |
+| bad-code-sanitization.js:6:11:6:25 | statements |
+| bad-code-sanitization.js:6:24:6:25 | [] |
+| bad-code-sanitization.js:7:21:7:70 | `${name ... key])}` |
+| bad-code-sanitization.js:7:31:7:43 | safeProp(key) |
+| bad-code-sanitization.js:8:27:8:36 | statements |
+| bad-code-sanitization.js:8:27:8:46 | statements.join(';') |
+| bad-code-sanitization.js:8:27:8:46 | statements.join(';') |
+| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) |
+| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) |
+| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) |
+| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
+| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
+| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
+| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
+| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
+| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
+| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
+| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
+| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
+edges
+| bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | bad-code-sanitization.js:7:31:7:43 | safeProp(key) |
+| bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` |
+| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` |
+| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` |
+| bad-code-sanitization.js:6:11:6:25 | statements | bad-code-sanitization.js:8:27:8:36 | statements |
+| bad-code-sanitization.js:6:24:6:25 | [] | bad-code-sanitization.js:6:11:6:25 | statements |
+| bad-code-sanitization.js:7:21:7:70 | `${name ... key])}` | bad-code-sanitization.js:6:24:6:25 | [] |
+| bad-code-sanitization.js:7:31:7:43 | safeProp(key) | bad-code-sanitization.js:7:21:7:70 | `${name ... key])}` |
+| bad-code-sanitization.js:8:27:8:36 | statements | bad-code-sanitization.js:8:27:8:46 | statements.join(';') |
+| bad-code-sanitization.js:8:27:8:36 | statements | bad-code-sanitization.js:8:27:8:46 | statements.join(';') |
+| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) |
+| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
+| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
+| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
+#select
+| bad-code-sanitization.js:8:27:8:46 | statements.join(';') | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | $@ flows to here and is used to construct code. | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | Improperly sanitized value |
+| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | Improperly sanitized value |
+| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | Improperly sanitized value |
+| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | Improperly sanitized value |
+| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | Improperly sanitized value |
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.qlref b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.qlref
new file mode 100644
index 00000000000..20c6dffd7b3
--- /dev/null
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.qlref
@@ -0,0 +1 @@
+Security/CWE-094/ImproperCodeSanitization.ql
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js
new file mode 100644
index 00000000000..21780a98138
--- /dev/null
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js
@@ -0,0 +1,45 @@
+function safeProp(key) {
+    return /^[_$a-zA-Z][_$a-zA-Z0-9]*$/.test(key) ? `.${key}` : `[${JSON.stringify(key)}]`;
+}
+
+function test1() {
+    const statements = [];
+    statements.push(`${name}${safeProp(key)}=${stringify(thing[key])}`);
+    return `(function(){${statements.join(';')}})` // NOT OK
+}
+
+import htmlescape from 'htmlescape'
+
+function test2(props) {
+    const pathname = props.data.pathname;
+    return `function(){return new Error('${htmlescape(pathname)}')}`; // NOT OK
+}
+
+function test3(input) {
+    return `(function(){${JSON.stringify(input)}))` // NOT OK
+}
+
+function evenSaferProp(key) {
+    return /^[_$a-zA-Z][_$a-zA-Z0-9]*$/.test(key) ? `.${key}` : `[${JSON.stringify(key)}]`.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, '');
+}
+
+function test4(input) {
+    return `(function(){${evenSaferProp(input)}))` // OK
+}
+
+function test4(input) {
+    var foo = `(function(){${JSON.stringify(input)}))` // OK - for this query - we can type-track to a code-injection sink.
+    setTimeout(foo);
+}
+
+function test5(input) {
+    console.log('methodName() => ' + JSON.stringify(input)); // OK
+}
+
+function test6(input) {
+    return `(() => {${JSON.stringify(input)})` // NOT OK
+}
+
+function test7(input) {
+    return `() => {${JSON.stringify(input)}` // NOT OK
+}
\ No newline at end of file

From 51426701381b8ab8779f64592bc5cb7f46f1c406 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Wed, 10 Jun 2020 22:30:45 +0200
Subject: [PATCH 2/6] don't import AdditionalSinks, refactor sink out in new
 HeuristicSinks instead

---
 .../CWE-094/ImproperCodeSanitization.ql       |  4 +--
 .../javascript/heuristics/AdditionalSinks.qll | 28 +++------------
 .../javascript/heuristics/HeuristicSinks.qll  | 35 +++++++++++++++++++
 3 files changed, 41 insertions(+), 26 deletions(-)
 create mode 100644 javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll

diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
index 87f97e8060b..8fd4cf3b94f 100644
--- a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
+++ b/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)))
diff --git a/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll b/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
index 64cfff5dd75..7d1920073b5 100644
--- a/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
+++ b/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() {
diff --git a/javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll b/javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll
new file mode 100644
index 00000000000..ae26e66b3b4
--- /dev/null
+++ b/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)\\].*")
+  }
+}

From aa3482cbaef6d82292e188a5a2529d845adbb8bc Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Wed, 10 Jun 2020 22:58:02 +0200
Subject: [PATCH 3/6] improve detection of duplicate results with
 `js/code-injection`

---
 .../CWE-094/ImproperCodeSanitization.ql       | 37 ++++++++++++++-----
 .../ImproperCodeSanitization.expected         | 14 +++++++
 .../CodeInjection/bad-code-sanitization.js    | 14 ++++++-
 3 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
index 8fd4cf3b94f..1da33050a20 100644
--- a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
+++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
@@ -11,7 +11,7 @@
  *       external/cwe/cwe-116
  */
 
-// TODO: Proper customizations module, Source class Sink class etc.
+// TODO: Proper customizations module, Source class Sink class etc. and qldoc.
 import javascript
 import DataFlow::PathGraph
 private import semmle.javascript.heuristics.HeuristicSinks
@@ -33,7 +33,7 @@ class Configuration extends TaintTracking::Configuration {
   }
 }
 
-private DataFlow::Node source() {
+private DataFlow::CallNode source() {
   result instanceof HtmlSanitizerCall
   or
   result = DataFlow::globalVarRef("JSON").getAMemberCall("stringify")
@@ -42,8 +42,7 @@ private DataFlow::Node source() {
 private StringOps::ConcatenationLeaf sink() {
   exists(StringOps::ConcatenationRoot root, int i |
     root.getOperand(i) = result and
-    not exists(result.getStringValue()) and
-    not root = endsInCodeInjectionSink()
+    not exists(result.getStringValue())
   |
     exists(StringOps::ConcatenationLeaf functionLeaf |
       functionLeaf = root.getOperand(any(int j | j < i))
@@ -56,19 +55,39 @@ private StringOps::ConcatenationLeaf sink() {
   )
 }
 
+DataFlow::SourceNode remoteFlow(DataFlow::TypeTracker t) {
+  t.start() and
+  result instanceof RemoteFlowSource
+  or
+  exists(DataFlow::TypeTracker t2 | result = remoteFlow(t2).track(t2, t))
+}
+
+DataFlow::SourceNode remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
+
 private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
   t.start() 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.
+  (
+    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)))
 }
 
-private DataFlow::Node endsInCodeInjectionSink() {
-  result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end())
+DataFlow::Node endsInCodeInjectionSink() {
+  result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end()) and
+  result.getFile().getBaseName() = "bad-code-sanitization.js" // TODO: TMp
 }
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
-where cfg.hasFlowPath(source, sink)
+where
+  cfg.hasFlowPath(source, sink) and
+  // Basic detection of duplicate results with `js/code-injection`.
+  not (
+    sink.getNode().(StringOps::ConcatenationLeaf).getRoot() = endsInCodeInjectionSink() and
+    remoteFlow().flowsTo(source.getNode().(DataFlow::InvokeNode).getAnArgument())
+  )
 select sink.getNode(), source, sink, "$@ flows to here and is used to construct code.",
   source.getNode(), "Improperly sanitized value"
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
index 26c13d2140d..79c97be21a7 100644
--- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
@@ -16,12 +16,21 @@ nodes
 | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
 | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
 | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
+| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) |
+| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) |
+| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) |
 | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
 | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
 | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
 | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
 | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
 | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
+| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) |
+| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) |
+| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
 edges
 | bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | bad-code-sanitization.js:7:31:7:43 | safeProp(key) |
 | bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` |
@@ -35,11 +44,16 @@ edges
 | bad-code-sanitization.js:8:27:8:36 | statements | bad-code-sanitization.js:8:27:8:46 | statements.join(';') |
 | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) |
 | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) |
+| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) |
 | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) |
 | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
+| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
 #select
 | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | $@ flows to here and is used to construct code. | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | Improperly sanitized value |
 | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | Improperly sanitized value |
 | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | Improperly sanitized value |
+| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | Improperly sanitized value |
 | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | Improperly sanitized value |
 | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | Improperly sanitized value |
+| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | Improperly sanitized value |
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js
index 21780a98138..ab5a5ff892d 100644
--- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js
@@ -28,7 +28,7 @@ function test4(input) {
 }
 
 function test4(input) {
-    var foo = `(function(){${JSON.stringify(input)}))` // OK - for this query - we can type-track to a code-injection sink.
+    var foo = `(function(){${JSON.stringify(input)}))` // NOT OK - we can type-track to a code-injection sink, the source is not remote flow.
     setTimeout(foo);
 }
 
@@ -42,4 +42,14 @@ function test6(input) {
 
 function test7(input) {
     return `() => {${JSON.stringify(input)}` // NOT OK
-}
\ No newline at end of file
+}
+
+var express = require('express');
+
+var app = express();
+
+app.get('/some/path', function(req, res) {
+  var foo = `(function(){${JSON.stringify(req.param("wobble"))}))` // NOT - the source is remote-flow, but we know of no sink.
+
+  setTimeout(`(function(){${JSON.stringify(req.param("wobble"))}))`); // OK - the source is remote-flow, and the sink is code-injection.
+});

From c375a0c61111008f5ce371a79c3e60d410de8282 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Thu, 11 Jun 2020 11:16:38 +0200
Subject: [PATCH 4/6] fix compilation and update expected output

---
 .../semmle/javascript/heuristics/AdditionalSinks.qll   |  2 +-
 .../CWE-094/CodeInjection/CodeInjection.expected       | 10 ++++++++++
 .../HeuristicSourceCodeInjection.expected              |  9 +++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll b/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
index 7d1920073b5..d1228c84248 100644
--- a/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
+++ b/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
@@ -19,7 +19,7 @@ private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomiz
 private import semmle.javascript.security.dataflow.InsecureRandomnessCustomizations
 private import HeuristicSinks as Sinks
 
-private class HeuristicSink = Sinks::HeuristicSink;
+class HeuristicSink = Sinks::HeuristicSink;
 
 private class HeuristicCodeInjectionSink extends Sinks::HeuristicCodeInjectionSink,
   CodeInjection::Sink { }
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected
index 6182246e929..7151a14f63e 100644
--- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected
@@ -64,6 +64,11 @@ nodes
 | angularjs.js:53:32:53:39 | location |
 | angularjs.js:53:32:53:46 | location.search |
 | angularjs.js:53:32:53:46 | location.search |
+| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
 | express.js:7:24:7:69 | "return ...  + "];" |
 | express.js:7:24:7:69 | "return ...  + "];" |
 | express.js:7:44:7:62 | req.param("wobble") |
@@ -193,6 +198,10 @@ edges
 | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
 | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
 | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
 | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ...  + "];" |
 | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ...  + "];" |
 | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ...  + "];" |
@@ -261,6 +270,7 @@ edges
 | angularjs.js:47:16:47:30 | location.search | angularjs.js:47:16:47:23 | location | angularjs.js:47:16:47:30 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:47:16:47:23 | location | User-provided value |
 | angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:29 | location | angularjs.js:50:22:50:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:50:22:50:29 | location | User-provided value |
 | angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:53:32:53:39 | location | User-provided value |
+| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | $@ flows to here and is interpreted as code. | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | User-provided value |
 | express.js:7:24:7:69 | "return ...  + "];" | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ...  + "];" | $@ flows to here and is interpreted as code. | express.js:7:44:7:62 | req.param("wobble") | User-provided value |
 | express.js:9:34:9:79 | "return ...  + "];" | express.js:9:54:9:72 | req.param("wobble") | express.js:9:34:9:79 | "return ...  + "];" | $@ flows to here and is interpreted as code. | express.js:9:54:9:72 | req.param("wobble") | User-provided value |
 | express.js:12:8:12:53 | "return ...  + "];" | express.js:12:28:12:46 | req.param("wobble") | express.js:12:8:12:53 | "return ...  + "];" | $@ flows to here and is interpreted as code. | express.js:12:28:12:46 | req.param("wobble") | User-provided value |
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected
index a4b7a3b1ea9..cb6966f4f11 100644
--- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected
@@ -64,6 +64,11 @@ nodes
 | angularjs.js:53:32:53:39 | location |
 | angularjs.js:53:32:53:46 | location.search |
 | angularjs.js:53:32:53:46 | location.search |
+| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
 | eslint-escope-build.js:20:22:20:22 | c |
 | eslint-escope-build.js:20:22:20:22 | c |
 | eslint-escope-build.js:21:16:21:16 | c |
@@ -197,6 +202,10 @@ edges
 | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
 | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
 | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
+| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
 | eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
 | eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
 | eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |

From adabd2daca91ba3b6fb96b37851cc527c7098211 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 12 Jun 2020 11:26:49 +0200
Subject: [PATCH 5/6] add qldoc and customizations module

---
 .../CWE-094/ImproperCodeSanitization.ql       | 57 +++++-----------
 .../dataflow/ImproperCodeSanitization.qll     | 27 ++++++++
 ...ImproperCodeSanitizationCustomizations.qll | 65 +++++++++++++++++++
 3 files changed, 107 insertions(+), 42 deletions(-)
 create mode 100644 javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
 create mode 100644 javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll

diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
index 1da33050a20..83695656aa5 100644
--- a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
+++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
@@ -11,59 +11,30 @@
  *       external/cwe/cwe-116
  */
 
-// TODO: Proper customizations module, Source class Sink class etc. and qldoc.
 import javascript
+import semmle.javascript.security.dataflow.ImproperCodeSanitization::ImproperCodeSanitization
 import DataFlow::PathGraph
 private import semmle.javascript.heuristics.HeuristicSinks
 private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
 
 /**
- * A taint-tracking configuration for reasoning about improper code sanitization vulnerabilities.
+ * Gets a type-tracked instance of `RemoteFlowSource` using type-tracker `t`.
  */
-class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "ImproperCodeSanitization" }
-
-  override predicate isSource(DataFlow::Node source) { source = source() }
-
-  override predicate isSink(DataFlow::Node sink) { sink = sink() }
-
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer instanceof StringReplaceCall // any string-replace that happens after the bad-sanitizer, is assumed to be a good sanitizer.
-    // TODO: Specialize? This regexp sanitizes: /[<>\b\f\n\r\t\0\u2028\u2029]/g
-  }
-}
-
-private DataFlow::CallNode source() {
-  result instanceof HtmlSanitizerCall
-  or
-  result = DataFlow::globalVarRef("JSON").getAMemberCall("stringify")
-}
-
-private StringOps::ConcatenationLeaf sink() {
-  exists(StringOps::ConcatenationRoot root, int i |
-    root.getOperand(i) = result and
-    not exists(result.getStringValue())
-  |
-    exists(StringOps::ConcatenationLeaf functionLeaf |
-      functionLeaf = root.getOperand(any(int j | j < i))
-    |
-      functionLeaf
-          .getStringValue()
-          .regexpMatch([".*function( )?([a-zA-Z0-9]+)?( )?\\(.*", ".*eval\\(.*",
-                ".*new Function\\(.*", "(^|.*[^a-zA-Z0-9])\\(.*\\)( )?=>.*"])
-    )
-  )
-}
-
-DataFlow::SourceNode remoteFlow(DataFlow::TypeTracker t) {
+private DataFlow::SourceNode remoteFlow(DataFlow::TypeTracker t) {
   t.start() and
   result instanceof RemoteFlowSource
   or
   exists(DataFlow::TypeTracker t2 | result = remoteFlow(t2).track(t2, t))
 }
 
-DataFlow::SourceNode remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
+/**
+ * Gets a type-tracked reference to a `RemoteFlowSource`.
+ */
+private DataFlow::SourceNode remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
 
+/**
+ * Gets a type-back-tracked instance of a code-injection sink using type-tracker `t`.
+ */
 private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
   t.start() and
   (
@@ -76,9 +47,11 @@ private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
   exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, endsInCodeInjectionSink(t2)))
 }
 
-DataFlow::Node endsInCodeInjectionSink() {
-  result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end()) and
-  result.getFile().getBaseName() = "bad-code-sanitization.js" // TODO: TMp
+/**
+ * Gets a reference to to a data-flow node that ends in a code-injection sink.
+ */
+private DataFlow::Node endsInCodeInjectionSink() {
+  result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end())
 }
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
new file mode 100644
index 00000000000..43276a27461
--- /dev/null
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
@@ -0,0 +1,27 @@
+/**
+ * Provides a taint-tracking configuration for reasoning about improper code
+ * sanitization.
+ *
+ * Note, for performance reasons: only import this file if
+ * `ImproperCodeSanitization::Configuration` is needed, otherwise
+ * `ImproperCodeSanitizationCustomizations` should be imported instead.
+ */
+
+import javascript
+
+module ImproperCodeSanitization {
+  import ImproperCodeSanitizationCustomizations::ImproperCodeSanitization
+
+  /**
+   * A taint-tracking configuration for reasoning about improper code sanitization vulnerabilities.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "ImproperCodeSanitization" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof Sanitizer }
+  }
+}
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll
new file mode 100644
index 00000000000..dbd067d2e26
--- /dev/null
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll
@@ -0,0 +1,65 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * improper code sanitization, as well as extension points for
+ * adding your own.
+ */
+
+import javascript
+
+module ImproperCodeSanitization {
+  /**
+   * A data flow source for improper code sanitization.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for improper code sanitization.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for improper code sanitization.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A call to a HTML sanitizer seen as a source for improper code sanitization
+   */
+  class HtmlSanitizerCallAsSource extends Source {
+    HtmlSanitizerCallAsSource() { this instanceof HtmlSanitizerCall }
+  }
+
+  /**
+   * A call to `JSON.stringify()` seen as a source for improper code sanitization
+   */
+  class JSONStringifyAsSource extends Source {
+    JSONStringifyAsSource() { this = DataFlow::globalVarRef("JSON").getAMemberCall("stringify") }
+  }
+
+  /**
+   * A leaf in a string-concatenation, where the string-concatenation constructs code that looks like a function.
+   */
+  class FunctionStringConstruction extends Sink, StringOps::ConcatenationLeaf {
+    FunctionStringConstruction() {
+      exists(StringOps::ConcatenationRoot root, int i |
+        root.getOperand(i) = this and
+        not exists(this.getStringValue())
+      |
+        exists(StringOps::ConcatenationLeaf functionLeaf |
+          functionLeaf = root.getOperand(any(int j | j < i))
+        |
+          functionLeaf
+              .getStringValue()
+              .regexpMatch([".*function( )?([a-zA-Z0-9]+)?( )?\\(.*", ".*eval\\(.*",
+                    ".*new Function\\(.*", "(^|.*[^a-zA-Z0-9])\\(.*\\)( )?=>.*"])
+        )
+      )
+    }
+  }
+
+  /**
+   * A call to `String.prototype.replace` seen as a sanitizer for improper code sanitization.
+   * All calls to replace that happens after the initial improper sanitization is seen as a sanitizer.
+   */
+  class StringReplaceCallAsSanitizer extends Sanitizer, StringReplaceCall { }
+}

From f0ec2eb37b857b0dc97040119b48abccc2e1e438 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 12 Jun 2020 11:47:53 +0200
Subject: [PATCH 6/6] add missing qldoc

---
 .../javascript/security/dataflow/ImproperCodeSanitization.qll  | 3 +++
 .../dataflow/ImproperCodeSanitizationCustomizations.qll        | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
index 43276a27461..ebe7c1b834f 100644
--- a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
@@ -9,6 +9,9 @@
 
 import javascript
 
+/**
+ * Classes and predicates for reasoning about improper code sanitization.
+ */
 module ImproperCodeSanitization {
   import ImproperCodeSanitizationCustomizations::ImproperCodeSanitization
 
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll
index dbd067d2e26..82679705020 100644
--- a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll
@@ -6,6 +6,9 @@
 
 import javascript
 
+/**
+ * Classes and predicates for reasoning about improper code sanitization.
+ */
 module ImproperCodeSanitization {
   /**
    * A data flow source for improper code sanitization.