Is .text() safe or not to sanitize data? [JQuery]

4k views Asked by At

I've seen that this question has been asked elsewhere:

Escaping HTML strings with jQuery

The answer marked as correct by @travis says that .text() is fine. However, some people mentionned in the commentaries (e. g. @nivcaner and @lior) that this solution is not good. Where do we stand? Can I safely use it?

I'm working on a web application where an user can create a document in his browser where tables, headers and videos can be added via JQuery. When the user "saves" his document, its structure is translated into an array, where all elements of the array consist of "pure" text, with no tags (e.g. there is one array element for each table cell which contains the text of that cell, etc.). Then the array is converted into JSON format and is sent to the server and is processed / sanitized / saved in JSON format via PHP.

After that, the document can be accessed by other users, and this of course introduces potential security holes. The idea is that they read the JSON data from the server and then recreate the document via javascript/JQuery on the client side.

While I'm pretty confident that my PHP code does its job of properly sanitizing the JSON data, I'm afraid of some attack where a malicious user could trick other users into reading JSON data from an untrusted source. For this reason, I have the feeling that it's better to also validate any JSON data coming from the server before re-creating tables, etc. on the client side. Assuming that the variable currentCellText holds the (JSON read) text to be written in the table cell #mytd, can I safely use the code $("#mytd").text(currentCellText) to re-create the cell contents?

1

There are 1 answers

5
Patrick Roberts On BEST ANSWER

From jQuery .text() article:

We need to be aware that this method escapes the string provided as necessary so that it will render correctly in HTML. To do so, it calls the DOM method .createTextNode(), does not interpret the string as HTML.

This means that the data gets stored to a text node, which by specification cannot render the stored text as HTML, so using $("#mytd").text(currentCellText) should be safe.

The 1.x line of jQuery's implementation supports this article. When you look at the uncompressed source for jQuery 1.11.3, you'll find this:

jQuery.fn.extend({
    text: function( value ) {
        return access( this, function( value ) {
            return value === undefined ?
                jQuery.text( this ) :
                this.empty().append( ( this[0] && this[0].ownerDocument || document ).createTextNode( value ) );
        }, null, value, arguments.length );
    },
    ...

The relevant line being this:

this.empty().append( ( this[0] && this[0].ownerDocument || document ).createTextNode( value ) );

Most of the line is simply locating the correct document with which to create the text node, but the value which is the passed in string, will never directly be interpreted as HTML.

Also, looking in the uncompressed code for jQuery 2.1.4, you'll find that the implementation uses the .textContent attribute for assignment, which relies on browser implementation for a complete sanitization of the passed text, rather than partial:

jQuery.fn.extend({
    text: function( value ) {
        return access( this, function( value ) {
            return value === undefined ?
                jQuery.text( this ) :
                this.empty().each(function() {
                    if ( this.nodeType === 1 || this.nodeType === 11 || this.nodeType === 9 ) {
                        this.textContent = value;
                    }
                });
        }, null, value, arguments.length );
    },
    ...

The relevant part of this code is here:

this.empty().each(function() {
    if ( this.nodeType === 1 || this.nodeType === 11 || this.nodeType === 9 ) {
        this.textContent = value;
    }
});

As this is what executes when an argument is passed to the .text() function.