Merge branch 'js-team-sprint' into js/insecure-http-options

Author: esbena

change-notes/1.25/analysis-javascript.md

@@ -36,7 +36,12 @@
 | Incomplete HTML attribute sanitization (`js/incomplete-html-attribute-sanitization`) | security, external/cwe/cwe-20, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights potential XSS vulnerabilities due to incomplete sanitization of HTML meta-characters. Results are shown on LGTM by default. |
 | Unsafe expansion of self-closing HTML tag (`js/unsafe-html-expansion`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights potential XSS vulnerabilities caused by unsafe expansion of self-closing HTML tags. |
 | Unsafe shell command constructed from library input (`js/shell-command-constructed-from-input`) | correctness, security, external/cwe/cwe-078, external/cwe/cwe-088 | Highlights potential command injections due to a shell command being constructed from library inputs. Results are shown on LGTM by default. |
+| Download of sensitive file through insecure connection (`js/insecure-download`) | security, external/cwe/cwe-829 | Highlights downloads of sensitive files through an unencrypted protocol. Results are shown on LGTM by default. |
+| Exposure of private files (`js/exposure-of-private-files`) | security, external/cwe/cwe-200 | Highlights servers that serve private files. Results are shown on LGTM by default. |
+| Creating biased random numbers from a cryptographically secure source (`js/biased-cryptographic-random`) | security, external/cwe/cwe-327 | Highlights mathematical operations on cryptographically secure numbers that can create biased results. Results are shown on LGTM by default. |
+| Storage of sensitive information in build artifact (`js/build-artifact-leak`) | security, external/cwe/cwe-312 | Highlights storage of sensitive information in build artifacts. Results are shown on LGTM by default. |
 | Improper code sanitization (`js/bad-code-sanitization`) | security, external/cwe/cwe-094, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights string concatenation where code is constructed without proper sanitization. Results are shown on LGTM by default. |
+| Resource exhaustion (`js/resource-exhaustion`) | security, external/cwe/cwe-770 | Highlights operations that may cause the resources of the application to be exhausted. Results are shown on LGTM by default. |
 | Disabling certificate validation (`js/disabling-certificate-validation`) | security, external/cwe-295 | Highlights locations where SSL certificate validation is disabled. Results are shown on LGTM by default. | 
 
 ## Changes to existing queries

javascript/config/suites/javascript/security

@@ -45,6 +45,7 @@
 + semmlecode-javascript-queries/Security/CWE-730/RegExpInjection.ql: /Security/CWE/CWE-730
 + semmlecode-javascript-queries/Security/CWE-754/UnvalidatedDynamicMethodCall.ql: /Security/CWE/CWE-754
 + semmlecode-javascript-queries/Security/CWE-770/MissingRateLimiting.ql: /Security/CWE/CWE-770
++ semmlecode-javascript-queries/Security/CWE-770/ResourceExhaustion.ql: /Security/CWE/CWE-770
 + semmlecode-javascript-queries/Security/CWE-776/XmlBomb.ql: /Security/CWE/CWE-776
 + semmlecode-javascript-queries/Security/CWE-798/HardcodedCredentials.ql: /Security/CWE/CWE-798
 + semmlecode-javascript-queries/Security/CWE-807/ConditionalBypass.ql: /Security/CWE/CWE-807

javascript/ql/src/Security/CWE-200/PrivateFileExposure.qhelp

@@ -5,20 +5,33 @@
 
 <overview>
   <p>
-    Placeholder
+    Libraries like <code>express</code> provide easy methods for serving entire 
+    directories of static files from a web server.
+    However, using these can sometimes lead to accidental information exposure. 
+    If for example the <code>node_modules</code> folder is served, then an attacker
+    can access the <code>_where</code> field from a <code>package.json</code> file, 
+    which gives access to the absolute path of the file. 
   </p>
 </overview>
 
 <recommendation>
   <p>
-    Placeholder
+    Limit which folders of static files are served from a web server.
   </p>
 </recommendation>
 
 <example>
   <p>
-    Placeholder
+    In the example below, all the files from the <code>node_modules</code> are served. 
+    This allows clients to easily access all the files inside that folder,
+    which includes potentially private information inside <code>package.json</code> files.
   </p>
+  <sample src="examples/PrivateFileExposure.js"/>
+  <p>
+    The issue has been fixed below by only serving specific folders within the
+    <code>node_modules</code> folder.
+  </p>
+  <sample src="examples/PrivateFileExposureFixed.js"/>
 </example>
 
 <references>

javascript/ql/src/Security/CWE-200/PrivateFileExposure.ql

@@ -69,19 +69,33 @@ pragma[noinline]
 Folder getAPackageJSONFolder() { result = any(PackageJSON json).getFile().getParentContainer() }
 
 /**
- * Gets a reference to `dirname` that might cause information to be leaked.
- * That can happen if there is a `package.json` file in the same folder.
- * (It is assumed that the presence of a `package.json` file means that a `node_modules` folder can also exist.
+ * Gets a reference to `dirname`, the home folder, the current working folder, or the root folder.
+ * All of these might cause information to be leaked.
+ *
+ * For `dirname` that can happen if there is a `package.json` file in the same folder.
+ * It is assumed that the presence of a `package.json` file means that a `node_modules` folder can also exist.
+ *
+ * For the root/home/working folder, they contain so much information that they must leak information somehow (e.g. ssh keys in the `~/.ssh` folder).
  */
-DataFlow::Node dirname() {
+DataFlow::Node getALeakingFolder(string description) {
   exists(ModuleScope ms | result.asExpr() = ms.getVariable("__dirname").getAnAccess()) and
-  result.getFile().getParentContainer() = getAPackageJSONFolder()
+  result.getFile().getParentContainer() = getAPackageJSONFolder() and
+  description = "the folder " + result.getFile().getParentContainer().getRelativePath()
+  or
+  result = DataFlow::moduleImport("os").getAMemberCall("homedir") and
+  description = "the home folder "
+  or
+  result.mayHaveStringValue("/") and
+  description = "the root folder"
+  or
+  result.getStringValue() = [".", "./"] and
+  description = "the current working folder"
   or
-  result.getAPredecessor() = dirname()
+  result.getAPredecessor() = getALeakingFolder(description)
   or
   exists(StringOps::ConcatenationRoot root | root = result |
     root.getNumOperand() = 2 and
-    root.getOperand(0) = dirname() and
+    root.getOperand(0) = getALeakingFolder(description) and
     root.getOperand(1).getStringValue() = "/"
   )
 }
@@ -94,18 +108,17 @@ DataFlow::Node getAPrivateFolderPath(string description) {
     result = getANodeModulePath(path) and description = "the folder \"" + path + "\""
   )
   or
-  result = dirname() and
-  description = "the folder " + result.getFile().getParentContainer().getRelativePath()
-  or
-  result.getStringValue() = [".", "./"] and
-  description = "the current working folder"
+  result = getALeakingFolder(description)
 }
 
 /**
  * Gest a call that serves the folder `path` to the public.
  */
 DataFlow::CallNode servesAPrivateFolder(string description) {
-  result = DataFlow::moduleMember("express", "static").getACall() and
+  result = DataFlow::moduleMember(["express", "connect"], "static").getACall() and
+  result.getArgument(0) = getAPrivateFolderPath(description)
+  or
+  result = DataFlow::moduleImport("serve-static").getACall() and
   result.getArgument(0) = getAPrivateFolderPath(description)
 }
 

javascript/ql/src/Security/CWE-200/examples/PrivateFileExposure.js

@@ -0,0 +1,6 @@
+
+var express = require('express');
+
+var app = express();
+
+app.use('/node_modules', express.static(path.resolve(__dirname, '../node_modules')));

javascript/ql/src/Security/CWE-200/examples/PrivateFileExposureFixed.js

@@ -0,0 +1,7 @@
+
+var express = require('express');
+
+var app = express();
+
+app.use("jquery", express.static('./node_modules/jquery/dist'));
+app.use("bootstrap", express.static('./node_modules/bootstrap/dist'));

javascript/ql/src/Security/CWE-312/BuildArtifactLeak.qhelp

@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      Sensitive information included in a build artifact can allow an attacker to access
+      the sensitive information if the artifact is published.
+    </p>
+  </overview>
+
+  <recommendation>
+    <p>
+      Only store information that is meant to be publicly available in a build artifact.
+    </p>
+  </recommendation>
+
+  <example>
+    <p>
+      The following example creates a <code>webpack</code> configuration that inserts all environment
+      variables from the host into the build artifact:
+    </p>
+    <sample src="examples/build-leak.js"/>
+    <p>
+      The environment variables might include API keys or other sensitive information, and the build-system
+      should instead insert only the environment variables that are supposed to be public. 
+    </p>
+    <p>
+      The issue has been fixed below, where only the <code>DEBUG</code> environment variable is inserted into the artifact.
+    </p>
+    <sample src="examples/build-leak-fixed.js"/>
+  </example>
+  <references>
+    <li>webpack: <a href="https://webpack.js.org/plugins/define-plugin/">DefinePlugin API</a>.</li>
+  </references>
+</qhelp>

javascript/ql/src/Security/CWE-312/BuildArtifactLeak.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Storage of sensitive information in build artifact
+ * @description Including sensitive information in a build artifact can
+ *              expose it to an attacker.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id js/build-artifact-leak
+ * @tags security
+ *       external/cwe/cwe-312
+ *       external/cwe/cwe-315
+ *       external/cwe/cwe-359
+ */
+
+import javascript
+import semmle.javascript.security.dataflow.BuildArtifactLeak::BuildArtifactLeak
+import DataFlow::PathGraph
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "Sensitive data returned by $@ is stored in a build artifact here.", source.getNode(),
+  source.getNode().(CleartextLogging::Source).describe()

javascript/ql/src/Security/CWE-312/examples/build-leak-fixed.js

@@ -0,0 +1,9 @@
+const webpack = require("webpack");
+
+module.exports = [{
+    plugins: [
+        new webpack.DefinePlugin({
+            'process.env': JSON.stringify({ DEBUG: process.env.DEBUG })
+        })
+    ]
+}];

javascript/ql/src/Security/CWE-312/examples/build-leak.js

@@ -0,0 +1,9 @@
+const webpack = require("webpack");
+
+module.exports = [{
+    plugins: [
+        new webpack.DefinePlugin({
+            "process.env": JSON.stringify(process.env)
+        })
+    ]
+}];