hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-02 12:15:17 +02:00
Code Issues Packages Projects Releases Wiki Activity

add consistency checking for CWE-079

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2020-07-06 13:42:35 +02:00
parent dc8042adeb
commit c986f3bb7c
13 changed files with 1140 additions and 1137 deletions
Download Patch File Download Diff File Expand all files Collapse all files

0
javascript/ql/test/query-tests/Security/CWE-079/Consistency.expected Normal file
View File

9
javascript/ql/test/query-tests/Security/CWE-079/Consistency.ql Normal file
View File

@@ -0,0 +1,9 @@
import javascript
import testUtilities.ConsistencyChecking
import semmle.javascript.security.dataflow.DomBasedXss as DomXss
import semmle.javascript.security.dataflow.ReflectedXss as ReflectedXss
import semmle.javascript.security.dataflow.StoredXss as StoredXss
import semmle.javascript.security.dataflow.XssThroughDom as ThroughDomXss
import semmle.javascript.security.dataflow.ExceptionXss as ExceptionXss
import semmle.javascript.security.dataflow.UnsafeJQueryPlugin as UnsafeJqueryPlugin

40
javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss.expected
View File

@@ -86,16 +86,16 @@ nodes
| exception-xss.js:180:26:180:30 | error |
| exception-xss.js:182:19:182:23 | error |
| exception-xss.js:182:19:182:23 | error |
| tst.js:304:9:304:16 | location |
| tst.js:304:9:304:16 | location |
| tst.js:305:10:305:10 | e |
| tst.js:306:20:306:20 | e |
| tst.js:306:20:306:20 | e |
| tst.js:311:10:311:17 | location |
| tst.js:311:10:311:17 | location |
| tst.js:313:10:313:10 | e |
| tst.js:314:20:314:20 | e |
| tst.js:314:20:314:20 | e |
| tst.js:301:9:301:16 | location |
| tst.js:301:9:301:16 | location |
| tst.js:302:10:302:10 | e |
| tst.js:303:20:303:20 | e |
| tst.js:303:20:303:20 | e |
| tst.js:308:10:308:17 | location |
| tst.js:308:10:308:17 | location |
| tst.js:310:10:310:10 | e |
| tst.js:311:20:311:20 | e |
| tst.js:311:20:311:20 | e |
edges
| exception-xss.js:2:6:2:28 | foo | exception-xss.js:9:11:9:13 | foo |
| exception-xss.js:2:6:2:28 | foo | exception-xss.js:15:9:15:11 | foo |
@@ -178,14 +178,14 @@ edges
| exception-xss.js:180:10:180:22 | req.params.id | exception-xss.js:180:26:180:30 | error |
| exception-xss.js:180:26:180:30 | error | exception-xss.js:182:19:182:23 | error |
| exception-xss.js:180:26:180:30 | error | exception-xss.js:182:19:182:23 | error |
| tst.js:304:9:304:16 | location | tst.js:305:10:305:10 | e |
| tst.js:304:9:304:16 | location | tst.js:305:10:305:10 | e |
| tst.js:305:10:305:10 | e | tst.js:306:20:306:20 | e |
| tst.js:305:10:305:10 | e | tst.js:306:20:306:20 | e |
| tst.js:311:10:311:17 | location | tst.js:313:10:313:10 | e |
| tst.js:311:10:311:17 | location | tst.js:313:10:313:10 | e |
| tst.js:313:10:313:10 | e | tst.js:314:20:314:20 | e |
| tst.js:313:10:313:10 | e | tst.js:314:20:314:20 | e |
| tst.js:301:9:301:16 | location | tst.js:302:10:302:10 | e |
| tst.js:301:9:301:16 | location | tst.js:302:10:302:10 | e |
| tst.js:302:10:302:10 | e | tst.js:303:20:303:20 | e |
| tst.js:302:10:302:10 | e | tst.js:303:20:303:20 | e |
| tst.js:308:10:308:17 | location | tst.js:310:10:310:10 | e |
| tst.js:308:10:308:17 | location | tst.js:310:10:310:10 | e |
| tst.js:310:10:310:10 | e | tst.js:311:20:311:20 | e |
| tst.js:310:10:310:10 | e | tst.js:311:20:311:20 | e |
#select
| exception-xss.js:11:18:11:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:11:18:11:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |
| exception-xss.js:17:18:17:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:17:18:17:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |
@@ -203,5 +203,5 @@ edges
| exception-xss.js:155:18:155:18 | e | exception-xss.js:146:12:146:28 | document.location | exception-xss.js:155:18:155:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:146:12:146:28 | document.location | Exception text |
| exception-xss.js:175:18:175:18 | e | exception-xss.js:146:12:146:28 | document.location | exception-xss.js:175:18:175:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:146:12:146:28 | document.location | Exception text |
| exception-xss.js:182:19:182:23 | error | exception-xss.js:180:10:180:22 | req.params.id | exception-xss.js:182:19:182:23 | error | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:180:10:180:22 | req.params.id | Exception text |
| tst.js:306:20:306:20 | e | tst.js:304:9:304:16 | location | tst.js:306:20:306:20 | e | $@ is reinterpreted as HTML without escaping meta-characters. | tst.js:304:9:304:16 | location | Exception text |
| tst.js:314:20:314:20 | e | tst.js:311:10:311:17 | location | tst.js:314:20:314:20 | e | $@ is reinterpreted as HTML without escaping meta-characters. | tst.js:311:10:311:17 | location | Exception text |
| tst.js:303:20:303:20 | e | tst.js:301:9:301:16 | location | tst.js:303:20:303:20 | e | $@ is reinterpreted as HTML without escaping meta-characters. | tst.js:301:9:301:16 | location | Exception text |
| tst.js:311:20:311:20 | e | tst.js:308:10:308:17 | location | tst.js:311:20:311:20 | e | $@ is reinterpreted as HTML without escaping meta-characters. | tst.js:308:10:308:17 | location | Exception text |

2
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXssGood3.js
View File

@@ -136,7 +136,7 @@ app.get('/user/:id', function (req, res) {
res.send(escapeHtml1(url)); // OK
res.send(escapeHtml2(url)); // OK
res.send(escapeHtml3(url)); // OK - but FP
res.send(escapeHtml3(url)); // OK - but FP [INCONSISTENCY]
res.send(escapeHtml4(url)); // OK
});

2
javascript/ql/test/query-tests/Security/CWE-079/UnsafeJQueryPlugin.expected
View File

@@ -238,6 +238,6 @@ edges
| unsafe-jquery-plugin.js:127:6:127:19 | options.target | unsafe-jquery-plugin.js:126:33:126:39 | options | unsafe-jquery-plugin.js:127:6:127:19 | options.target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:126:14:128:3 | functio ... OK\\n\\t\\t} | '$.fn.my_plugin' plugin |
| unsafe-jquery-plugin.js:132:5:132:18 | options.target | unsafe-jquery-plugin.js:131:34:131:40 | options | unsafe-jquery-plugin.js:132:5:132:18 | options.target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:131:15:133:2 | functio ... T OK\\n\\t} | '$.fn.affix' plugin |
| unsafe-jquery-plugin.js:136:5:136:29 | options ... elector | unsafe-jquery-plugin.js:135:36:135:42 | options | unsafe-jquery-plugin.js:136:5:136:29 | options ... elector | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:135:17:137:2 | functio ... T OK\\n\\t} | '$.fn.tooltip' plugin |
| unsafe-jquery-plugin.js:157:44:157:59 | options.target.a | unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:157:44:157:59 | options.target.a | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:153:19:158:2 | functio ... gged\\n\\t} | '$.fn.my_plugin' plugin |
| unsafe-jquery-plugin.js:157:44:157:59 | options.target.a | unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:157:44:157:59 | options.target.a | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:153:19:158:2 | functio ... NCY]\\n\\t} | '$.fn.my_plugin' plugin |
| unsafe-jquery-plugin.js:170:6:170:11 | target | unsafe-jquery-plugin.js:160:38:160:44 | options | unsafe-jquery-plugin.js:170:6:170:11 | target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:160:19:173:2 | functio ... \\t\\t}\\n\\n\\t} | '$.fn.my_plugin' plugin |
| unsafe-jquery-plugin.js:179:5:179:18 | options.target | unsafe-jquery-plugin.js:178:27:178:33 | options | unsafe-jquery-plugin.js:179:5:179:18 | options.target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:178:18:180:2 | functio ... T OK\\n\\t} | '$.fn.my_plugin' plugin |

1158
javascript/ql/test/query-tests/Security/CWE-079/Xss.expected
View File

File diff suppressed because it is too large Load Diff

1024
javascript/ql/test/query-tests/Security/CWE-079/XssWithAdditionalSources.expected
View File

File diff suppressed because it is too large Load Diff

6
javascript/ql/test/query-tests/Security/CWE-079/exception-xss.js
View File

@@ -26,7 +26,7 @@
try {
unknown({ prop: foo });
} catch (e) {
$('myId').html(e); // NOT OK!
$('myId').html(e); // NOT OK! - but not detected due to not tainting object that have a tainted propety. [INCONSISTENCY]
}
try {
@@ -179,9 +179,9 @@ app.get('/user/:id', function (req, res) {
app.get('/user/:id', function (req, res) {
unknown(req.params.id, (error, res) => {
if (error) {
$('myId').html(error); // OK (falls through to the next statement)
$('myId').html(error); // NOT OK
}
$('myId').html(res); // NOT OK!
$('myId').html(res); // OK - does not contain an error, and `res` is otherwise unknown.
});
});

2
javascript/ql/test/query-tests/Security/CWE-079/partial.js
View File

@@ -49,7 +49,7 @@ app.get("/return", (req, res) => {
let callback = getFirst.bind(null, req.url);
res.send(callback); // OK - the callback itself is not tainted
res.send(callback()); // NOT OK - but not currently detected
res.send(callback()); // NOT OK - but not currently detected [INCONSISTENCY]
res.send(getFirst("Hello")); // OK - argument is not tainted from this call site
});

2
javascript/ql/test/query-tests/Security/CWE-079/sanitiser.js
View File

@@ -16,7 +16,7 @@ function test() {
var tainted = window.name;
var elt = document.createElement();
elt.innerHTML = "<a href=\"" + escapeAttr(tainted) + "\">" + escapeHtml(tainted) + "</a>"; // OK
elt.innerHTML = "<div>" + escapeAttr(tainted) + "</div>"; // NOT OK, but not flagged
elt.innerHTML = "<div>" + escapeAttr(tainted) + "</div>"; // NOT OK, but not flagged - [INCONSISTENCY]
const regex = /[<>'"&]/;
if (regex.test(tainted)) {

18
javascript/ql/test/query-tests/Security/CWE-079/tst.js
View File

@@ -11,17 +11,14 @@ function test() {
// NOT OK
$('<div style="width:' + target + 'px">');
// OK
$('<div style="width:' + +target + 'px">');
$('<div style="width:' + parseInt(target) + 'px">');
$('<div style="width:' + +target + 'px">'); // OK
$('<div style="width:' + parseInt(target) + 'px">'); // OK
// NOT OK
let params = (new URL(document.location)).searchParams;
$('name').html(params.get('name'));
$('name').html(params.get('name')); // NOT OK
// NOT OK
var searchParams = new URLSearchParams(target.substring(1));
$('name').html(searchParams.get('name'));
$('name').html(searchParams.get('name')); // NOT OK
}
function foo(target) {
@@ -331,14 +328,11 @@ function getTaintedUrl() {
}
function URLPseudoProperties() {
// NOT OK
let params = getTaintedUrl().searchParams;
$('name').html(params.get('name'));
$('name').html(params.get('name')); // NOT OK
// OK (.get is not defined on a URL)
let myUrl = getTaintedUrl();
$('name').html(myUrl.get('name'));
$('name').html(myUrl.get('name')); // OK (.get is not defined on a URL)
}

2
javascript/ql/test/query-tests/Security/CWE-079/typeahead.js
View File

@@ -7,7 +7,7 @@
source: autocompleter.ttAdapter(),
templates: {
suggestion: function(loc) {
return loc; // NOT OK!
return loc; // NOT OK! - but not flagged due to not connecting the Bloodhound source with this sink [INCONSISTENCY]
}
}
})

12
javascript/ql/test/query-tests/Security/CWE-079/unsafe-jquery-plugin.js
View File

@@ -65,7 +65,7 @@
$.fn.my_plugin = function my_plugin(element, options) {
this.$element = $(element);
this.options = $.extend({}, options);
if (this.options.parent) this.$parent = $(this.options.parent) // NOT OK
if (this.options.parent) this.$parent = $(this.options.parent) // NOT OK - but not flagged [INCONSISTENCY]
};
$.fn.my_plugin = function my_plugin(options) {
@@ -103,7 +103,7 @@
menu: '<div></div>',
target: '.my_plugin'
}, options);
$(options.menu); // OK
$(options.menu); // OK - but is flagged [INCONSISTENCY]
$(options.target); // NOT OK
};
@@ -113,7 +113,7 @@
};
$.fn.my_plugin = function my_plugin(options) {
options = $.extend({}, $.fn.my_plugin.defaults, options);
$(options.menu); // OK
$(options.menu); // OK - but is flagged [INCONSISTENCY]
$(options.target); // NOT OK
};
@@ -152,9 +152,9 @@
$.fn.my_plugin = function my_plugin(options) {
let target = options.target;
target === DEFAULTS.target? $(target): $(document).find(target); // NOT OK
options.target === DEFAULTS.target? $(options.target): $(document).find(options.target); // NOT OK
options.targets.a === DEFAULTS.target? $(options.target.a): $(document).find(options.target.a); // OK - but still flagged
target === DEFAULTS.target? $(target): $(document).find(target); // NOT OK - but not flagged [INCONSISTENCY]
options.target === DEFAULTS.target? $(options.target): $(document).find(options.target); // NOT OK - but not flagged [INCONSISTENCY]
options.targets.a === DEFAULTS.target? $(options.target.a): $(document).find(options.target.a); // OK - but still flagged [INCONSISTENCY]
}
$.fn.my_plugin = function my_plugin(options) {