mistic100 / jQuery-QueryBuilder

jQuery plugin offering an interface to create complex queries
https://querybuilder.js.org
MIT License
1.68k stars 552 forks source link

Checkmarx reports Client Potential XSS #905

Closed richardf closed 3 years ago

richardf commented 3 years ago

Our infosec area ran Checkmarx against an application using QueryBuilder which reported two potential XSS, as shown below. I am not sure those are real problems. Could they be false positives?

Client Potential XSS\Path 1:

The application's function embeds untrusted data in the generated output with html, at line 1452 of js/query-builder.standalone.js. This untrusted data is embedded straight into the output without proper sanitization or encoding, enabling an attacker to inject malicious code into the output.

Source File: js/query-builder.standalone.js Line: 3152 Object: change

Destination File: js/query-builder.standalone.js Line: 1464 Object: html

Code snippet Method: QueryBuilder.prototype.getRuleFilterSelect = function(rule, filters) {

...
3152. return this.change('getRuleFilterSelect', h, rule,
$.parseHTML(filters));

Method: QueryBuilder.prototype.createRuleFilters = function(rule) {

...
1464.
rule.$el.find(QueryBuilder.selectors.filter_container).html(htmlFilter);

Client Potential XSS\Path 2:

The application's function embeds untrusted data in the generated output with append, at line 1178 of js/query-builder.standalone.js. This untrusted data is embedded straight into the output without proper sanitization or encoding, enabling an attacker to inject malicious code into the output.

Source File: js/query-builder.standalone.js Line: 3096 Object: change

Destination File: js/query-builder.standalone.js Line: 1184 Object: append

Code snippet Method: QueryBuilder.prototype.getGroupTemplate = function(group_id, level) {

...
3096. return this.change('getGroupTemplate', h, level);

Method: QueryBuilder.prototype.setRoot = function(addRule, data, flags) {

...
1184. this.$el.append($group);
mistic100 commented 3 years ago

Is using $.parseHTML enough ?

mistic100 commented 3 years ago

Using $($.parseHTML(content)) instead of $(content) seems to solve it.

Tested locally by registering this handler :

$('#builder').on('getRuleFilterSelect.queryBuilder.filter', function(e) {
    e.value += '<script';
    e.value += '>alert("hello");</';
    e.value += 'script>';
  });

I will make the same change on all HTML parsing

richardf commented 3 years ago

Thank you for looking into this!