Adds support for npm and xsjslib modules

javascript/frameworks/xsjs/ext/xsjs.model.yml

@@ -60,6 +60,7 @@ extensions:
       extensible: sinkModel
     data:
       - [WebResponse, "Member[setBody].Argument[0]", html-injection]
+      - [XsjsDollar, "Member[import].Argument[0..1]", path-injection]
       # - [Mail, "Member[send].Argument[this]", "???"]
       # - [SMTPConnection, "Member[send].Argument[0]", "???"]
       # - ["HTTPClient", "Member[request].Argument[0]", "???"]
@@ -68,4 +69,5 @@ extensions:
       pack: codeql/javascript-all
       extensible: summaryModel
     data:
-      - [global, "Member[JSON].Member[parse]", "Argument[0]", "ReturnValue", taint]
+      - [global, "Member[JSON].Member[parse]", "Argument[0]", "ReturnValue", taint]
+      - ["@sap/xss-secure", "Member[encodeCSS,encodeHTML,encodeJS,encodeURL,encodeXML]", "Argument[0]", "ReturnValue", "taint"]

javascript/frameworks/xsjs/lib/advanced_security/javascript/frameworks/xsjs/AsyncXSJS.qll

@@ -1,23 +1,24 @@
 import javascript
 import DataFlow
+import XSJSLibModules
 
 /**
  * The root XSJS namespace, accessed as a dollar sign (`$`) symbol.
  */
-class XSJSDollarNamespace extends GlobalVarRefNode {
-  XSJSDollarNamespace() {
-    this = globalVarRef("$") and
-    this.getFile().getExtension() = "xsjs"
+class XSJSDollarNode extends DataFlow::SourceNode {
+  XSJSDollarNode() {
+    this.accessesGlobal("$") and
+    this.getFile().getExtension() = ["xsjs", "xsjslib"]
   }
 }
 
 /**
- * `TypeModel` for `XSJSDollarNamespace`.
+ * `TypeModel` for `XSJSDollarNode`.
  */
 class XSJSDollarTypeModel extends ModelInput::TypeModel {
   override DataFlow::Node getASource(string type) {
     type = "XsjsDollar" and
-    result = any(XSJSDollarNamespace dollar)
+    result = any(XSJSDollarNode dollar)
   }
 
   /**
@@ -37,9 +38,9 @@ class XSJSRequestOrResponse extends SourceNode instanceof PropRef {
   XSJSRequestOrResponse() {
     fieldName = ["request", "response"] and
     (
-      exists(XSJSDollarNamespace dollar | this = dollar.getAPropertyReference(fieldName))
+      exists(XSJSDollarNode dollar | this = dollar.getAPropertyReference(fieldName))
       or
-      exists(XSJSDollarNamespace dollar |
+      exists(XSJSDollarNode dollar |
         this =
           dollar
               .getAPropertyReference(fieldName)
@@ -171,7 +172,7 @@ class XSJSDatabaseConnectionReference extends MethodCallNode {
   string subNamespace;
 
   XSJSDatabaseConnectionReference() {
-    exists(XSJSDollarNamespace dollar |
+    exists(XSJSDollarNode dollar |
       this.getMethodName() = "getConnection" and
       this.getReceiver().getALocalSource() = dollar.getAPropertyReference(subNamespace)
     )
@@ -211,7 +212,7 @@ class XSJSHDBConnectionReference extends XSJSDatabaseConnectionReference {
 
 class XSJSUtilNamespace extends SourceNode instanceof PropRef {
   XSJSUtilNamespace() {
-    exists(XSJSDollarNamespace dollar | this = dollar.getAPropertyReference("util"))
+    exists(XSJSDollarNode dollar | this = dollar.getAPropertyReference("util"))
   }
 }
 

javascript/frameworks/xsjs/lib/advanced_security/javascript/frameworks/xsjs/XSJSLibModules.qll

@@ -0,0 +1,61 @@
+/** Provides classes for working with XSJSLib modules. */
+
+import javascript
+
+/**
+ * An XSJSLib module.
+ */
+class XSJSModule extends Module {
+  XSJSModule() { this.getFile().getExtension() = ["xsjs", "xsjslib"] }
+
+  /**
+   * Get a value that is explicitly exported from this module with under `name`.
+   */
+  override DataFlow::ValueNode getAnExportedValue(string name) {
+    exists(FunctionDeclStmt fds |
+      fds.getParent() = this.getTopLevel() and
+      fds.getName() = name and
+      result.getAstNode() = fds
+    )
+  }
+}
+
+/**
+ * An XSJSLib module import declaration.
+ * ```
+ * $.import("module.xsjslib");
+ * ```
+ */
+class XSJSImportExpr extends CallExpr, Import {
+  XSJSImportExpr() {
+    this.getReceiver().(GlobalVarAccess).getName() = "$" and
+    this.getFile().getExtension() = ["xsjs", "xsjslib"] and
+    this.getCalleeName() = "import"
+  }
+
+  override XSJSModule getEnclosingModule() { result = this.getTopLevel() }
+
+  override PathExpr getImportedPath() { result = this.getLastArgument() }
+
+  override DataFlow::Node getImportedModuleNode() { result = DataFlow::valueNode(this) }
+}
+
+private class XSJSModuleImportPath extends PathExpr, ConstantString {
+  XSJSModuleImportPath() { this = any(XSJSImportExpr e).getLastArgument() }
+
+  override Folder getSearchRoot(int priority) {
+    priority = 0 and
+    result = this.getFile().getParentContainer()
+  }
+
+  override string getValue() {
+    exists(XSJSImportExpr e |
+      this = e.getArgument(0) and result = this.getStringValue()
+      or
+      this = e.getArgument(1) and
+      result =
+        e.getArgument(0).getStringValue().replaceAll(".", "/") + "/" + this.getStringValue() +
+          ".xsjslib"
+    )
+  }
+}

javascript/frameworks/xsjs/lib/advanced_security/javascript/frameworks/xsjs/XSJSReflectedXssQuery.qll

@@ -13,6 +13,15 @@ class XSJSResponseSetBodyCall extends MethodCallNode {
   XSJSResponse getParentXSJSResponse() { result = response }
 }
 
+SourceNode xssSecure(TypeTracker t) {
+  t.start() and
+  result = moduleImport("@sap/xss-secure")
+  or
+  exists(TypeTracker t2 | result = xssSecure(t2).track(t2, t))
+}
+
+SourceNode xssSecure() { result = xssSecure(TypeTracker::end()) }
+
 class Configuration extends TaintTracking::Configuration {
   Configuration() { this = "XSJS Reflected XSS Query" }
 
@@ -35,4 +44,12 @@ class Configuration extends TaintTracking::Configuration {
       )
     )
   }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof DomBasedXss::Sanitizer or
+    node =
+      xssSecure()
+          .getAMemberInvocation(["encodeCSS", "encodeHTML", "encodeJS", "encodeURL", "encodeXML"])
+  }
 }

javascript/frameworks/xsjs/test/codeql-pack.lock.yml

@@ -5,16 +5,22 @@ dependencies:
     version: 1.1.2
   codeql/javascript-all:
     version: 2.0.0
+  codeql/javascript-queries:
+    version: 1.2.0
   codeql/mad:
     version: 1.0.8
   codeql/regex:
     version: 1.0.8
   codeql/ssa:
     version: 1.0.8
+  codeql/suite-helpers:
+    version: 1.0.8
   codeql/tutorial:
     version: 1.0.8
   codeql/typetracking:
     version: 1.0.8
+  codeql/typos:
+    version: 1.0.8
   codeql/util:
     version: 1.0.8
   codeql/xml:

javascript/frameworks/xsjs/test/models/modules/npm/npm.expected

@@ -0,0 +1,2 @@
+| npm.xsjs:3:14:3:71 | crypto. ... 1024 }) |
+| npm.xsjs:5:15:5:72 | crypto. ... 4096 }) |

javascript/frameworks/xsjs/test/models/modules/npm/npm.ql

@@ -0,0 +1,4 @@
+import javascript
+
+from CryptographicKeyCreation crypto
+select crypto

javascript/frameworks/xsjs/test/models/modules/npm/npm.xsjs

@@ -0,0 +1,5 @@
+const crypto = $.require("crypto");
+
+const bad1 = crypto.generateKeyPairSync("rsa", { modulusLength: 1024 }); // NOT OK
+
+const good1 = crypto.generateKeyPairSync("rsa", { modulusLength: 4096 }); // OK

javascript/frameworks/xsjs/test/qlpack.yml

@@ -4,5 +4,6 @@ version: 0.1.0
 extractor: javascript
 dependencies:
   codeql/javascript-all: "^2.0.0"
+  codeql/javascript-queries: "^1.2.0"
   advanced-security/javascript-sap-async-xsjs-queries: "^0.1.0"
   advanced-security/javascript-sap-async-xsjs-lib: "^0.1.0"

javascript/frameworks/xsjs/test/queries/XSJSLibSqlInjection/XSJSLibSqlInjection.expected

@@ -0,0 +1,3 @@
+nodes
+edges
+#select

javascript/frameworks/xsjs/test/queries/XSJSLibSqlInjection/XSJSLibSqlInjection.qlref

@@ -0,0 +1 @@
+XSJSSqlInjection/XSJSSqlInjection.ql

javascript/frameworks/xsjs/test/queries/XSJSLibSqlInjection/XSJSLibSqlInjection.xsjs

@@ -0,0 +1,11 @@
+var requestParameters = $.request.parameters;
+var param = JSON.parse(requestParameters.get("param"));
+
+let t1=$.import("lib/injection1.xsjslib");
+t1.test1(requestParameters);
+
+$.import("lib/injection2.xsjslib");
+$.lib.injection2.test2(param);
+
+let t3=$.import("lib.test3","injection3");
+t3.test3(param);

javascript/frameworks/xsjs/test/queries/XSJSLibSqlInjection/lib/injection1.xsjslib

@@ -0,0 +1,8 @@
+function test1(requestParameters) {
+  let query = "INSERT INTO " + requestParameters + ".ENTITY (COL1) VALUES (" + requestParameters + ")";
+
+  let dbConnection = $.db.getConnection();
+  let preparedStatement = dbConnection.prepareStatement(query);
+  preparedStatement.executeUpdate();
+  dbConnection.commit();
+}

javascript/frameworks/xsjs/test/queries/XSJSLibSqlInjection/lib/injection2.xsjslib

@@ -0,0 +1,8 @@
+function test2(requestParameters) {
+  let query = "INSERT INTO " + requestParameters + ".ENTITY (COL1) VALUES (" + requestParameters + ")";
+
+  let dbConnection = $.db.getConnection();
+  let preparedStatement = dbConnection.prepareStatement(query);
+  preparedStatement.executeUpdate();
+  dbConnection.commit();
+}

javascript/frameworks/xsjs/test/queries/XSJSLibSqlInjection/lib/test3/injection3.xsjslib

@@ -0,0 +1,8 @@
+function test3(requestParameters) {
+  let query = "INSERT INTO " + requestParameters + ".ENTITY (COL1) VALUES (" + requestParameters + ")";
+
+  let dbConnection = $.db.getConnection();
+  let preparedStatement = dbConnection.prepareStatement(query);
+  preparedStatement.executeUpdate();
+  dbConnection.commit();
+}

javascript/frameworks/xsjs/test/queries/XSJSReflectedXss/XSJSReflectedXss.expected

@@ -23,6 +23,13 @@ nodes
 | XSJSReflectedXss.xsjs:32:22:32:65 | request ... Value3) |
 | XSJSReflectedXss.xsjs:32:22:32:65 | request ... Value3) |
 | XSJSReflectedXss.xsjs:32:46:32:64 | someParameterValue3 |
+| XSJSReflectedXss.xsjs:45:7:45:67 | someParameterValue4 |
+| XSJSReflectedXss.xsjs:45:29:45:67 | request ... eter4") |
+| XSJSReflectedXss.xsjs:45:29:45:67 | request ... eter4") |
+| XSJSReflectedXss.xsjs:47:22:47:87 | request ... alue4)) |
+| XSJSReflectedXss.xsjs:47:22:47:87 | request ... alue4)) |
+| XSJSReflectedXss.xsjs:47:46:47:86 | xssSecu ... Value4) |
+| XSJSReflectedXss.xsjs:47:67:47:85 | someParameterValue4 |
 edges
 | XSJSReflectedXss.xsjs:11:7:11:67 | someParameterValue1 | XSJSReflectedXss.xsjs:13:46:13:64 | someParameterValue1 |
 | XSJSReflectedXss.xsjs:11:7:11:67 | someParameterValue1 | XSJSReflectedXss.xsjs:13:46:13:64 | someParameterValue1 |
@@ -44,6 +51,11 @@ edges
 | XSJSReflectedXss.xsjs:31:29:31:67 | request ... eter3") | XSJSReflectedXss.xsjs:31:7:31:67 | someParameterValue3 |
 | XSJSReflectedXss.xsjs:32:46:32:64 | someParameterValue3 | XSJSReflectedXss.xsjs:32:22:32:65 | request ... Value3) |
 | XSJSReflectedXss.xsjs:32:46:32:64 | someParameterValue3 | XSJSReflectedXss.xsjs:32:22:32:65 | request ... Value3) |
+| XSJSReflectedXss.xsjs:45:7:45:67 | someParameterValue4 | XSJSReflectedXss.xsjs:47:67:47:85 | someParameterValue4 |
+| XSJSReflectedXss.xsjs:45:29:45:67 | request ... eter4") | XSJSReflectedXss.xsjs:45:7:45:67 | someParameterValue4 |
+| XSJSReflectedXss.xsjs:45:29:45:67 | request ... eter4") | XSJSReflectedXss.xsjs:45:7:45:67 | someParameterValue4 |
+| XSJSReflectedXss.xsjs:47:46:47:86 | xssSecu ... Value4) | XSJSReflectedXss.xsjs:47:22:47:87 | request ... alue4)) |
+| XSJSReflectedXss.xsjs:47:46:47:86 | xssSecu ... Value4) | XSJSReflectedXss.xsjs:47:22:47:87 | request ... alue4)) |
+| XSJSReflectedXss.xsjs:47:67:47:85 | someParameterValue4 | XSJSReflectedXss.xsjs:47:46:47:86 | xssSecu ... Value4) |
 #select
-| XSJSReflectedXss.xsjs:13:22:13:65 | request ... Value1) | XSJSReflectedXss.xsjs:11:29:11:67 | request ... eter1") | XSJSReflectedXss.xsjs:13:22:13:65 | request ... Value1) | Reflected XSS vulnerability due to $@. | XSJSReflectedXss.xsjs:11:29:11:67 | request ... eter1") | user-provided value |
-
+| XSJSReflectedXss.xsjs:13:22:13:65 | request ... Value1) | XSJSReflectedXss.xsjs:11:29:11:67 | request ... eter1") | XSJSReflectedXss.xsjs:13:22:13:65 | request ... Value1) | Reflected XSS vulnerability due to $@. | XSJSReflectedXss.xsjs:11:29:11:67 | request ... eter1") | user-provided value |

javascript/frameworks/xsjs/test/queries/XSJSReflectedXss/XSJSReflectedXss.xsjs

@@ -10,12 +10,12 @@
 function test1(requestParameters) {
   let someParameterValue1 = requestParameters.get("someParameter1");
   $.response.contentType = "text/html";
-  $.response.setBody(requestParameterHandler(someParameterValue1));
+  $.response.setBody(requestParameterHandler(someParameterValue1)); // js/xsjs-reflected-xss
   $.response.status = $.net.http.OK;
 }
 
 /**
- * False positive case: content type is "text/plain"
+ * True negative case: content type is "text/plain"
  */
 function test2(requestParameters) {
   let someParameterValue2 = requestParameters.get("someParameter2");
@@ -25,7 +25,7 @@
 }
 
 /**
- * False positive case: content type is not set
+ * True negative case: content type is not set
  */
 function test3(requestParameters) {
   let someParameterValue3 = requestParameters.get("someParameter3");
@@ -36,3 +36,16 @@
 test1(requestParameters);
 test2(requestParameters);
 test3(requestParameters);
+
+/**
+ * True negative case: the value is sanitized
+ */
+var xssSecure = $.require('@sap/xss-secure');
+function test4(requestParameters) {
+  let someParameterValue4 = requestParameters.get("someParameter4");
+  $.response.contentType = "text/html";
+  $.response.setBody(requestParameterHandler(xssSecure.encodeHTML(someParameterValue4)));
+  $.response.status = $.net.http.OK;
+}
+
+test4(requestParameters);