Creating tables dynamically using jQuery does not work in WET v4.0.44+

[adam2]

Hi,

I've found a bug in WET v4.0.44+. I initially reported this bug to the CDTS group, but I can reproduce it using standard WET files as well.
wet-boew/cdts-sgdc#434

The issue is that when using jQuery to dynamically create an HTML table, it doesn't work in WET 4.0.44+. Some "tr" and "td" elements you would expect to be added are not being added. It's as if the jQuery append function isn't working properly, or something is stripping those elements from the DOM after they are added. It is difficult to explain, so I made a demo page which make the problem more obvious. Please see the attached HTML file. The page is based on the WET documentation index page which is using v4.0.47.2. This bug did not exist in WET 4.0.43.

Please let me know if you have any questions.

Thanks,
Adam
wet-jquery-tables-bug.html.txt

[duboisp]

Yes and that is intentional.

As documented in the release note of v4.0.44, in the section "Special notes": <tr>, <td> and other tags need to be inserted by using the javascript DOM interface. And you can read in the subsequent bullet that all jQuery DOM manipulation are sanitized with DOMPurify.

Since v4.0.44 we patched directly jQuery which has improved the security not just of wet-boew but also to any custom javascript code that leverage jQuery.

If you want to discuss further, please join us at one of our weekly live support from 13h to 16h Estern time every Tuesday afternoon. You will find the meeting link here: https://github.com/wet-boew/wet-boew/wiki/WET-BOEW-Code-sprint

Cheers

[adam2]

So you've intentionally broken jQuery and labeled it as "wontfix"? Thanks for your help.

Now we are forced to fix thousands of lines of code or not upgrade WET to 4.0.44.

[duboisp]

@adam2 it was a security patch to prevent XSS attack.

If you want, we can discuss more about your use case during one of our weekly Tuesday afternoon live support. Or alternatively you can find my contact information in GCDirectory and we will coordinate a meeting to discuss further.

[duboisp]

@adam2 I didn't saw you at our weekly office hours 😞.

Have you tried to use a different and more recent version of jQuery to run with your custom code while keeping our patched version. I do think the data table plugin are compatible with jQuery 3.x.

[adam2]

Hi,

I'm sorry, I've been busy with other work. For now our team is satisfied staying on WET/CDTS 4.0.43. We will have to find a new solution in the future, if the bug is not fixed in WET. This changes breaks 3 applications that we know about, one of them very seriously.

I'm not sure what the solution will entail, but I think we will need to override or remove the DOMPurify fix one way or another. Maybe using a more modern version of jQuery to get their XSS fix, but who knows what else that will break in WET. Hopefully we can do all this while still using CDTS. If not, the scope of the problem for us increases.

Adam

[adam2]

For future refence to anyone viewing this bug:

This is the jQuery XSS bug which I think the WET team were trying to fix when they broke jQuery (CVE-2020-11023):
GHSA-jpcq-cgw6-v4j6

This is the commit that caused the bug:
9308d36

This is the bug in DOMPurify which removes tr/td elements, where the DOMPurify people say they won't/can't fix it either:
cure53/DOMPurify#324

[jpardey]

I've just encountered this issue while upgrading to v4.0.44. In one place, I was simply able to use innerHTML instead, but this takes away much of the usability of jQuery in many other settings. I'll need to do a lot more testing.

It would be great if there was some way to opt out of this, for data known to be fully sanitized, or known to not take any input from the URL or similar.

[adam2]

Hi,

I found this workaround which is simple enough to implement on our pages. The script block has to be placed in-between the jquery.min.js script and the wet-boew.js script. Maybe you want to put it in its own script file.

I tested it with IE11, Edge, Firefox and Chrome. The premise is that you use Object.defineProperty to define a getter/setter for the jQuery functions that WET-BOEW is overriding, but the setter will not let WET update the function/property. This removes most of the jQuery overrides that WET has implemented, but not all of them. You can still use DOMPurify as well, if you want to sanitize untrusted code yourself.

<script>
(function(){
	const jQueryOriginalFunctions = {
		append:      jQuery.fn.append, 
		prepend:     jQuery.fn.prepend, 
		before:      jQuery.fn.before, 
		after:       jQuery.fn.after, 
		replaceWith: jQuery.fn.replaceWith, 
		init:        jQuery.fn.init,
		html:        jQuery.html
	};
	// Prevent WET-BOEW for overriding these functions without causing a TypeError in the WET-BOEW code
	Object.defineProperty(jQuery.fn, "append", {get:function(){return jQueryOriginalFunctions.append;}, set:function(){}});
	Object.defineProperty(jQuery.fn, "prepend", {get:function(){return jQueryOriginalFunctions.prepend;}, set:function(){}});
	Object.defineProperty(jQuery.fn, "before", {get:function(){return jQueryOriginalFunctions.before;}, set:function(){}});
	Object.defineProperty(jQuery.fn, "after", {get:function(){return jQueryOriginalFunctions.after;}, set:function(){}});
	Object.defineProperty(jQuery.fn, "replaceWith", {get:function(){return jQueryOriginalFunctions.replaceWith;}, set:function(){}});
	Object.defineProperty(jQuery, "html", {get:function(){return jQueryOriginalFunctions.html;}, set:function(){}});
})();
</script>

There are shorter methods I looked at, like using Object.assign, but that doesn't work in IE11. I also looked at
Object.freeze, but that generates errors in the WET code because they are using "strict" mode. The above method allows WET to fail silently and keeps jQuery working as intended.

[wewhite]

I see that there is an exception if using wb-tables DataTables plugin.
If appending any one of "<tbody/>", "<tr/>", "<td />", "<td/>" tags, then DOMPurify.sanitize() is not executed.
However, I do not believe those are valid tags.
Should the array of allowed tags be [<tbody>", "<tr>", "<td>", "<td>"] and also [</tbody>", "</tr>", "</td>", "</td>"]

This was committed here: 581e165

[pearl-nguyen]

I am upgrading our intranet apps from wet v4.043 to v4.0.75, both tables of class table and w-tables do not allow me to add new row dynamically using jquery. Any or tags are removed even if I remove class table or w-tables from the table tag.

Has this issue been fixed?

Thanks

[pearl-nguyen]

this statement

		$('#checkin-selection-table-body').append($('<tr>')
			.append($('<td>').append("text1"))
			.append($('<td>').append("text2"))
			.append($('<td>').append("text3"))
			.append($('<td>').append("text4"))
			.append($('<td>').append("text5")))

generate empty tbody

[mercury64]

after modifiying:

	dataTableAllowedTag = [
		"<tbody>", "<tr>", "<td>", "<td>",
		"</tbody>", "</tr>", "</td>", "</td>"
	],

Then wait for wb-tables to be ready:

        $(document).on("wb-ready.wb", function(event) {
            console.log("WB Ready");
            $('#checkin-selection-table-body').append($('<tr>')
                .append($('<td>').append("text1"))
                .append($('<td>').append("text2"))
                .append($('<td>').append("text3"))
                .append($('<td>').append("text4"))
                .append($('<td>').append("text5")));
        });

A row is added.

I'm not sure why dataTableAllowedTag is not being fixed correctly. As I stated above, the allowed tag array looks incorrect, and does not filter correctly.

[mercury64]

this statement

generate empty tbody

I submitted a pull request.

#9746

[pearl-nguyen]

@mercury64 your proposed fix works . Thanks

[pearl-nguyen]

Can we also update jquery-fix to fix the following case:

$("#tbody").append("<tr><td>a</td><td>a</td><td>a</td><td>a</td><td>a</td></tr>")

Change to dataTableAllowedTag does not help to fix this case.

This above proposed solution #9293 (comment) does fix this issue

<script> (function(){ const jQueryOriginalFunctions = { append: jQuery.fn.append, prepend: jQuery.fn.prepend, before: jQuery.fn.before, after: jQuery.fn.after, replaceWith: jQuery.fn.replaceWith, init: jQuery.fn.init, html: jQuery.html }; // Prevent WET-BOEW for overriding these functions without causing a TypeError in the WET-BOEW code Object.defineProperty(jQuery.fn, "append", {get:function(){return jQueryOriginalFunctions.append;}, set:function(){}}); Object.defineProperty(jQuery.fn, "prepend", {get:function(){return jQueryOriginalFunctions.prepend;}, set:function(){}}); Object.defineProperty(jQuery.fn, "before", {get:function(){return jQueryOriginalFunctions.before;}, set:function(){}}); Object.defineProperty(jQuery.fn, "after", {get:function(){return jQueryOriginalFunctions.after;}, set:function(){}}); Object.defineProperty(jQuery.fn, "replaceWith", {get:function(){return jQueryOriginalFunctions.replaceWith;}, set:function(){}}); Object.defineProperty(jQuery, "html", {get:function(){return jQueryOriginalFunctions.html;}, set:function(){}}); })(); </script>

[mercury64]

Can we also update jquery-fix to fix the following case:

$("#tbody").append("<tr><td>a</td><td>a</td><td>a</td><td>a</td><td>a</td></tr>")

Change to dataTableAllowedTag does not help to fix this case.

I'd recommend using the DataTable API instead of appending directly onto the table.

This works for what you are trying to accomplish:

        $(document).on("wb-ready.wb", function(event) {
            console.log("WB Ready");

            var table = $('#checkin-selection-table-body').DataTable();
            var rowDataAAAAA = ["a", "a", "a", "a", "a"];
            var rowDataBBBBB = ["b", "b", "b", "b", "b"];

            table.row.add(rowDataAAAAA).draw();
            table.row.add(rowDataBBBBB).draw();

        });

[bozzit]

Not a big Javascript programmer but what if we did something like this instead. in the case of DataTables check if the tag is within the HTML not the HTML is equal to the tag. Could go a step further and not within but starts with?

dataTableAllowedTag = [
        "<tbody/>",
        "<tr/>",
        "<td />",
        "<td/>",
        "<tr>",
        "<td>"
    ],
    sanitize = function( html ) {
 
        // Add an exception for DataTable plugin
        if (window.DataTable && checkDataTableAllowedTags( html ) !== -1 ) {
            return html;
        }
 
        return DOMPurify.sanitize( html );
    };
 
    function checkDataTableAllowedTags(html)
    {
        for (const tag of dataTableAllowedTag)
        {
            if (html.indexOf(tag) !== -1)
            {
                return html.indexOf(tag);
            }
        }
        return -1;
    };

[mercury64]

> "<tbody/>",
>         "<tr/>",
>         "<td />",
>         "<td/>",
>         "<tr>",
>         "<td>"

The only tags that would be acceptable is tr and td.
The whole point of my pull request #9746 fixes the allowed tags.

How is that the HTML tags <tbody/>, <tr/>, <td />, <td/> are even allowable? Who writes HTML like this?

[bozzit]

I realize that those tags are weird but I left them in the code as they where already there didn't want to break some existing test/use case that may of prompted the authors to put those tags in, and I added tr and td, and a function that checks if the tags in that list are in the html before DOMPurify Sanitation.

<td />
is probably not unheard of when adding elements to DOM using jQuery, short form of
<td></td>

I replaced

if ( window.DataTable && dataTableAllowedTag.indexOf( html ) !== -1 ) {

with

if (window.DataTable && checkDataTableAllowedTags( html ) !== -1 ) {

and added the new checkDataTableAllowedTags function that checks for the existence of the Tag in html instead of checking for an exact match of the html = tag.

Your Pull request doesn't work in my case as the row.child method is appending

<tr><td></td></tr>

And that gets stripped out by DOMPurify as it doesn't match anything in dataTableAllowedTag Hence my proposed changes. Adding that full string to dataTableAllowedTag also solved my specific issue, but what else will DOMPurify break, in complex dataTable processing and extension.

<RANT>
I know that this may not be as secure, but DOMPurify breaking jQuery and jQuery.dataTables 
seems weird to me. Why write wet-boew with jQuery as a dependency, if you are going to prevent 
users from leveraging/using jQuery/Datatable in their pages.  Maybe wet-boew should of been 
written using some other framework?

I have jQuery at my finger tips on pages that use wet-boew, why would I now want to use j
avascript DOM Interface instead? seems like a step backwards to me.  All Examples provided 
by jQuery.Datatables use jQuery to perform various tasks and extend the dataTables such 
as filters, sorting, Details button, tfoot summaries etc... we now have to review all that 
code and figure out a javascript Alternative for things that are broken by this security measure?
</RANT>