import all the shared XSS sources and sinks

Author: erik-krogh

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

@@ -21,5 +21,5 @@ from
 where
   cfg.hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
-  sink.getNode().(Sink).getVulnerabilityKind() + " vulnerability due to $@.", source.getNode(),
+  sink.getNode().(XSS::Shared::Sink).getVulnerabilityKind() + " vulnerability due to $@.", source.getNode(),
   "user-provided value"

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

@@ -1,12 +1,18 @@
 /**
- * Provides a taint-tracking configuration for TODO:
+ * Provides a taint-tracking configuration for reasoning about cross-site 
+ * scripting vulnerabilities where the taint-flow passes through a thrown 
+ * exception. 
  */
 
 import javascript
 
 module ExceptionXss {
-  import Xss::DomBasedXss // imports sinks
-  import DomBasedXssCustomizations::DomBasedXss // imports sources
+  import DomBasedXssCustomizations::DomBasedXss as DomBasedXssCustom
+  import ReflectedXssCustomizations::ReflectedXss as ReflectedXssCustom 
+  import Xss::DomBasedXss as DomBasedXss 
+  import Xss::ReflectedXss as ReflectedXSS
+  import Xss::StoredXss as StoredXss
+  import Xss as XSS
 
   DataFlow::Node getExceptionalSuccssor(DataFlow::Node pred) {
     exists(DataFlow::FunctionNode func |
@@ -53,16 +59,16 @@ module ExceptionXss {
     Configuration() { this = "ExceptionXss"}
 
     override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
-      source instanceof Source and label instanceof NotYetThrown
+      source instanceof XSS::Shared::Source and label instanceof NotYetThrown
     }
 
     override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
-   	  sink instanceof Sink and label.isDataOrTaint()
+   	  sink instanceof XSS::Shared::Sink and label.isDataOrTaint()
    	}
 
     override predicate isSanitizer(DataFlow::Node node) {
       super.isSanitizer(node) or
-      node instanceof Sanitizer
+      node instanceof XSS::Shared::Sanitizer
     }
 
     override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl) {

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

@@ -6,7 +6,7 @@
 import javascript
 
 module ReflectedXss {
-  import Xss::ReflectedXss
+  import ReflectedXssCustomizations::ReflectedXss
 
   /**
    * A taint-tracking configuration for reasoning about XSS.
@@ -23,13 +23,4 @@ module ReflectedXss {
       node instanceof Sanitizer
     }
   }
-
-  /** A third-party controllable request input, considered as a flow source for reflected XSS. */
-  class ThirdPartyRequestInputAccessAsSource extends Source {
-    ThirdPartyRequestInputAccessAsSource() {
-      this.(HTTP::RequestInputAccess).isThirdPartyControllable()
-      or
-      this.(HTTP::RequestHeaderAccess).getAHeaderName() = "referer"
-    }
-  }
 }

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

@@ -0,0 +1,19 @@
+/**
+ * Provides default sources for reasoning about reflected
+ * cross-site scripting vulnerabilities.
+ */
+
+import javascript
+
+module ReflectedXss {
+  import Xss::ReflectedXss
+
+  /** A third-party controllable request input, considered as a flow source for reflected XSS. */
+  class ThirdPartyRequestInputAccessAsSource extends Source {
+    ThirdPartyRequestInputAccessAsSource() {
+      this.(HTTP::RequestInputAccess).isThirdPartyControllable()
+      or
+      this.(HTTP::RequestHeaderAccess).getAHeaderName() = "referer"
+    }
+  }
+}

javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss.expected

@@ -52,6 +52,12 @@ nodes
 | exception-xss.js:96:10:96:10 | e |
 | exception-xss.js:97:18:97:18 | e |
 | exception-xss.js:97:18:97:18 | e |
+| exception-xss.js:107:13:107:25 | req.params.id |
+| exception-xss.js:107:13:107:25 | req.params.id |
+| exception-xss.js:108:11:108:11 | e |
+| exception-xss.js:109:14:109:30 | "Exception: " + e |
+| exception-xss.js:109:14:109:30 | "Exception: " + e |
+| exception-xss.js:109:30:109:30 | e |
 edges
 | exception-xss.js:2:9:2:31 | foo | exception-xss.js:9:11:9:13 | foo |
 | exception-xss.js:2:9:2:31 | foo | exception-xss.js:15:9:15:11 | foo |
@@ -107,6 +113,11 @@ edges
 | exception-xss.js:95:12:95:14 | foo | exception-xss.js:95:11:95:22 | [foo, "bar"] |
 | exception-xss.js:96:10:96:10 | e | exception-xss.js:97:18:97:18 | e |
 | exception-xss.js:96:10:96:10 | e | exception-xss.js:97:18:97:18 | e |
+| exception-xss.js:107:13:107:25 | req.params.id | exception-xss.js:108:11:108:11 | e |
+| exception-xss.js:107:13:107:25 | req.params.id | exception-xss.js:108:11:108:11 | e |
+| exception-xss.js:108:11:108:11 | e | exception-xss.js:109:30:109:30 | e |
+| exception-xss.js:109:30:109:30 | e | exception-xss.js:109:14:109:30 | "Exception: " + e |
+| exception-xss.js:109:30:109:30 | e | exception-xss.js:109:14:109:30 | "Exception: " + e |
 #select
 | exception-xss.js:11:18:11:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:11:18:11:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |
 | exception-xss.js:17:18:17:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:17:18:17:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |

javascript/ql/test/query-tests/Security/CWE-079/exception-xss.js

@@ -97,3 +97,16 @@
 		$('myId').html(e); // NOT OK! 
 	}
 });
+
+var express = require('express');
+
+var app = express();
+
+app.get('/user/:id', function(req, res) {
+  try {
+    unknown(req.params.id);
+  } catch(e) {
+    res.send("Exception: " + e); // NOT OK!
+  }
+});
+