From 94dd9a1c04bc13b522b80c9ad828658188c83ffe Mon Sep 17 00:00:00 2001
From: Asger F <asger@semmle.com>
Date: Mon, 28 Oct 2019 16:59:43 +0000
Subject: [PATCH] JS: Block XSS flow through encodeURIComponent

---
 .../javascript/security/dataflow/Xss.qll       | 18 ++++++++++++++++++
 .../query-tests/Security/CWE-079/encodeuri.js  |  4 ++++
 2 files changed, 22 insertions(+)
 create mode 100644 javascript/ql/test/query-tests/Security/CWE-079/encodeuri.js

diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
index fbea366ed93..52c4a9b9332 100644
--- a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
@@ -39,6 +39,18 @@ module Shared {
       )
     }
   }
+
+  /**
+   * A call to `encodeURI` or `encodeURIComponent`, viewed as a sanitizer for
+   * XSS vulnerabilities.
+   */
+  class UriEncodingSanitizer extends Sanitizer, DataFlow::CallNode {
+    UriEncodingSanitizer() {
+      exists(string name | this = DataFlow::globalVarRef(name).getACall() |
+        name = "encodeURI" or name = "encodeURIComponent"
+      )
+    }
+  }
 }
 
 /** Provides classes and predicates for the DOM-based XSS query. */
@@ -251,6 +263,8 @@ module DomBasedXss {
    * so any such replacement stops taint propagation.
    */
   private class MetacharEscapeSanitizer extends Sanitizer, Shared::MetacharEscapeSanitizer { }
+
+  private class UriEncodingSanitizer extends Sanitizer, Shared::UriEncodingSanitizer { }
 }
 
 /** Provides classes and predicates for the reflected XSS query. */
@@ -294,6 +308,8 @@ module ReflectedXss {
    * so any such replacement stops taint propagation.
    */
   private class MetacharEscapeSanitizer extends Sanitizer, Shared::MetacharEscapeSanitizer { }
+
+  private class UriEncodingSanitizer extends Sanitizer, Shared::UriEncodingSanitizer { }
 }
 
 /** Provides classes and predicates for the stored XSS query. */
@@ -320,4 +336,6 @@ module StoredXss {
    * so any such replacement stops taint propagation.
    */
   private class MetacharEscapeSanitizer extends Sanitizer, Shared::MetacharEscapeSanitizer { }
+
+  private class UriEncodingSanitizer extends Sanitizer, Shared::UriEncodingSanitizer { }
 }
diff --git a/javascript/ql/test/query-tests/Security/CWE-079/encodeuri.js b/javascript/ql/test/query-tests/Security/CWE-079/encodeuri.js
new file mode 100644
index 00000000000..a48f720bed1
--- /dev/null
+++ b/javascript/ql/test/query-tests/Security/CWE-079/encodeuri.js
@@ -0,0 +1,4 @@
+function test() {
+  let loc = window.location.href;
+  $('<a href="' + encodeURIComponent(loc) + '">click</a>'); // OK
+}