Skip to content

Commit 33f98dd

Browse files
author
Esben Sparre Andreasen
committed
JS: add query: js/stored-xss
1 parent e2fac8a commit 33f98dd

File tree

9 files changed

+218
-0
lines changed

9 files changed

+218
-0
lines changed

‎javascript/config/suites/javascript/security‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
+ semmlecode-javascript-queries/Security/CWE-022/TaintedPath.ql: /Security/CWE/CWE-022
33
+ semmlecode-javascript-queries/Security/CWE-078/CommandInjection.ql: /Security/CWE/CWE-078
44
+ semmlecode-javascript-queries/Security/CWE-079/ReflectedXss.ql: /Security/CWE/CWE-079
5+
+ semmlecode-javascript-queries/Security/CWE-079/StoredXss.ql: /Security/CWE/CWE-079
56
+ semmlecode-javascript-queries/Security/CWE-079/Xss.ql: /Security/CWE/CWE-079
67
+ semmlecode-javascript-queries/Security/CWE-089/SqlInjection.ql: /Security/CWE/CWE-089
78
+ semmlecode-javascript-queries/Security/CWE-094/CodeInjection.ql: /Security/CWE/CWE-094

‎javascript/ql/src/Security/CWE-079/StoredXss.qhelp‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
9+
Directly using uncontrolled stored value (for example, file names) to
10+
create HTML content without properly sanitizing the input first,
11+
allows for a cross-site scripting vulnerability.
12+
13+
</p>
14+
<p>
15+
16+
This kind of vulnerability is also called <i>stored</i> cross-site
17+
scripting, to distinguish it from other types of cross-site scripting.
18+
19+
</p>
20+
</overview>
21+
22+
<recommendation>
23+
<p>
24+
25+
To guard against cross-site scripting, consider using contextual
26+
output encoding/escaping before using uncontrolled stored values to
27+
create HTML content, or one of the other solutions that are mentioned
28+
in the references.
29+
30+
</p>
31+
</recommendation>
32+
33+
<example>
34+
<p>
35+
36+
The following example code writes file names directly to a HTTP
37+
response. This leaves the website vulnerable to cross-site scripting,
38+
if an attacker can choose the file names on the disk.
39+
40+
</p>
41+
<sample src="examples/StoredXss.js" />
42+
<p>
43+
Sanitizing the file names prevents the vulnerability:
44+
</p>
45+
<sample src="examples/StoredXssGood.js" />
46+
</example>
47+
48+
<references>
49+
<li>
50+
OWASP:
51+
<a href="https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet">XSS
52+
(Cross Site Scripting) Prevention Cheat Sheet</a>.
53+
</li>
54+
<li>
55+
OWASP
56+
<a href="https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting">Types of Cross-Site
57+
Scripting</a>.
58+
</li>
59+
<li>
60+
Wikipedia: <a href="http://en.wikipedia.org/wiki/Cross-site_scripting">Cross-site scripting</a>.
61+
</li>
62+
</references>
63+
</qhelp>

‎javascript/ql/src/Security/CWE-079/StoredXss.ql‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Stored cross-site scripting
3+
* @description Using uncontrolled stored values in HTML allows for
4+
* a stored cross-site scripting vulnerability.
5+
* @kind problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id js/stored-xss
9+
* @tags security
10+
* external/cwe/cwe-079
11+
* external/cwe/cwe-116
12+
*/
13+
14+
import javascript
15+
import semmle.javascript.security.dataflow.StoredXss::StoredXss
16+
17+
from Configuration xss, DataFlow::Node source, DataFlow::Node sink
18+
where xss.hasFlow(source, sink)
19+
select sink, "Stored cross-site scripting vulnerability due to $@.",
20+
source, "stored value"

‎javascript/ql/src/Security/CWE-079/examples/StoredXss.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
var express = require('express'),
2+
fs = require('fs');
3+
4+
express().get('/list-directory', function(req, res) {
5+
fs.readdir('/public', function (error, fileNames) {
6+
var list = '<ul>';
7+
fileNames.forEach(fileName => {
8+
list += '<li>' + fileName '</li>'; // BAD: `fileName` can contain HTML elements
9+
});
10+
list += '</ul>'
11+
res.send(list);
12+
});
13+
});

‎javascript/ql/src/Security/CWE-079/examples/StoredXssGood.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
var express = require('express'),
2+
fs = require('fs'),
3+
escape = require('escape-html');
4+
5+
express().get('/list-directory', function(req, res) {
6+
fs.readdir('/public', function (error, fileNames) {
7+
var list = '<ul>';
8+
fileNames.forEach(fileName => {
9+
list += '<li>' + escape(fileName) '</li>'; // GOOD: escaped `fileName` can not contain HTML elements
10+
});
11+
list += '</ul>'
12+
res.send(list);
13+
});
14+
});

‎javascript/ql/src/semmle/javascript/security/dataflow/StoredXss.qll‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about stored
3+
* cross-site scripting vulnerabilities.
4+
*/
5+
6+
import javascript
7+
import semmle.javascript.security.dataflow.RemoteFlowSources
8+
import semmle.javascript.security.dataflow.ReflectedXss as ReflectedXss
9+
import semmle.javascript.security.dataflow.DomBasedXss as DomBasedXss
10+
11+
module StoredXss {
12+
/**
13+
* A data flow source for XSS vulnerabilities.
14+
*/
15+
abstract class Source extends DataFlow::Node { }
16+
17+
/**
18+
* A data flow sink for XSS vulnerabilities.
19+
*/
20+
abstract class Sink extends DataFlow::Node { }
21+
22+
/**
23+
* A sanitizer for XSS vulnerabilities.
24+
*/
25+
abstract class Sanitizer extends DataFlow::Node { }
26+
27+
/**
28+
* A taint-tracking configuration for reasoning about XSS.
29+
*/
30+
class Configuration extends TaintTracking::Configuration {
31+
Configuration() { this = "StoredXss" }
32+
33+
override predicate isSource(DataFlow::Node source) {
34+
source instanceof Source
35+
}
36+
37+
override predicate isSink(DataFlow::Node sink) {
38+
sink instanceof Sink
39+
}
40+
41+
override predicate isSanitizer(DataFlow::Node node) {
42+
super.isSanitizer(node) or
43+
node instanceof Sanitizer
44+
}
45+
}
46+
47+
/** A file name, considered as a flow source for stored XSS. */
48+
class FileNameSourceAsSource extends Source {
49+
FileNameSourceAsSource() {
50+
this instanceof FileNameSource
51+
}
52+
}
53+
54+
/** An ordinary XSS sink, considered as a flow sink for stored XSS. */
55+
class XssSinkAsSink extends Sink {
56+
XssSinkAsSink() {
57+
this instanceof ReflectedXss::ReflectedXss::Sink or
58+
this instanceof DomBasedXss::DomBasedXss::Sink
59+
}
60+
}
61+
62+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| xss-through-filenames.js:8:18:8:23 | files1 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:7:43:7:48 | files1 | stored value |
2+
| xss-through-filenames.js:26:19:26:24 | files1 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:25:43:25:48 | files1 | stored value |
3+
| xss-through-filenames.js:33:19:33:24 | files2 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:25:43:25:48 | files1 | stored value |
4+
| xss-through-filenames.js:37:19:37:24 | files3 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:25:43:25:48 | files1 | stored value |

‎javascript/ql/test/query-tests/Security/CWE-079/StoredXss.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-079/StoredXss.ql

‎javascript/ql/test/query-tests/Security/CWE-079/xss-through-filenames.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
var http = require('http');
2+
var fs = require('fs');
3+
4+
var express = require('express');
5+
6+
express().get('/', function(req, res) {
7+
fs.readdir("/myDir", function (error, files1) {
8+
res.send(files1); // NOT OK
9+
});
10+
});
11+
12+
/**
13+
* The essence of a real world vulnerability.
14+
*/
15+
http.createServer(function (req, res) {
16+
17+
function format(files2) {
18+
var files3 = [];
19+
files2.sort(sort).forEach(function (file) {
20+
files3.push('<li>' + file + '</li>');
21+
});
22+
return files3.join('');
23+
}
24+
25+
fs.readdir("/myDir", function (error, files1) {
26+
res.write(files1); // NOT OK
27+
28+
var dirs = [];
29+
var files2 = [];
30+
files1.forEach(function (file) {
31+
files2.push(file);
32+
});
33+
res.write(files2); // NOT OK
34+
35+
var files3 = format(files2);
36+
37+
res.write(files3); // NOT OK
38+
39+
});
40+
});

0 commit comments

Comments
 (0)