1

I use an AJAX .get() method to load HTML for a "Product Search" app and render it into a HTML node ($searchModalBody):

   productSearch: function productSearch(e, element) {
        var $searchModal = $('#product-search-modal');
        var $searchModalBody = $searchModal.find('.modal-body');
        var url = $searchModal.data('url');
        ...
        $.get(url, {
            fromQuote: true, customerNumber: custNumber, hasQtyField: false
        }).done(function (response) {   
            $searchModalBody.html(response);
        })

How can I sanitize the response to avoid any DOM XSS?

I've read that .text() is preferred over .html(), but since I am loading HTML, .text() will just escape all of it.

Sorry if I have missed anything. I've been trying to read up on XSS prevention, but couldn't find anything. I might just not fully understand what I need to be doing/looking for.

Moritz Ringler
  • 9,772
  • 9
  • 21
  • 34
Mint
  • 13
  • 4
  • 1
    so the HTML is coming from an untrusted source? – epascarello Mar 10 '23 at 19:07
  • injecting unsafe html is fine if done correctly, the issue most likely if any is that the user-supplied `search` param is not escaped in your returning HTML which is the the xss, its not in the code you have shown – Lawrence Cherone Mar 10 '23 at 19:22
  • @epascarello the HTML is coming from another solution's layout, which was used to build the product search application. We are using Coverity, and it flagged the .html() call as a potential risk. – Mint Mar 10 '23 at 20:22
  • [dompurify](https://www.npmjs.com/package/dompurify) is a common library to sanitize HTML input to remove XSS concerns. This question is related to react, but the same concepts apply - https://stackoverflow.com/a/29048207/864233 – romellem Mar 10 '23 at 21:09
  • I'd strongly recommend scraping the HTML you're retrieving down to the string and numeric primitives you need, and injecting those into your site instead of whole chunks of I-hope-the-sanitizer-worked HTML. Partly because it makes sanitizing easier and safer, partly because it makes your site less dependent on the structure of the other one. (If they do a redesign, you want your scraper to break so it can throw errors and alert you asap; if you're just dropping HTML in it might break the layout without you knowing about it until a user tells you) – Daniel Beck Mar 10 '23 at 22:09
  • ...though on rereading the comments: is the "product search application" another one of your organization's apps? i.e. not an untrusted external source? If so, you don't have to worry so much about XSS, but maybe you should talk to the product search team about setting up an API so you don't have to resort to scraping their site to fill yours... – Daniel Beck Mar 10 '23 at 22:13
  • Why does the API return HTML? Can't it return JSON and then you build the HTML based on it on your side? – Kaiido Mar 10 '23 at 23:42

1 Answers1

1

You should avoid using .html() and .innerHTML as it introduces security risks as you mentioned.

jquery documentation:

Do not use these methods to insert strings obtained from untrusted sources such as URL query parameters, cookies, or form inputs. Doing so can introduce cross-site-scripting (XSS) vulnerabilities. Remove or escape any user input before adding content to the document.

MDN on innerHTML:

Note: This is a security risk if the string to be inserted might contain potentially malicious content. When inserting user-supplied data you should always consider using Element.SetHTML() instead, in order to sanitize the content before it is inserted.

So, using setHTML (experimental) might be a little bit safer, demo:

const elm = document.querySelector('.demo');

elm.setHTML('<script>alert("XSS");</script\><h1>Oups!</h1>');
<div class="demo"></div>

Also, OWASP recommends this library:

HTML Sanitization will strip dangerous HTML from a variable and return a safe string of HTML. OWASP recommends DOMPurify for HTML Sanitization.

let clean = DOMPurify.sanitize(dirty);

There are some further things to consider:

  • If you sanitize content and then modify it afterwards, you can easily void your security efforts.
  • If you sanitize content and then send it to a library for use, check that it doesn’t mutate that string somehow. Otherwise, again, your security efforts are void. You must regularly patch DOMPurify or other HTML Sanitization libraries that you use.
  • Browsers change functionality and bypasses are being discovered regularly.

Note that Sanitizer object that setHTML uses is still experimental

const sanitizer = new Sanitizer();

const theRawHTML = '<script>alert("XSS");</script\><h1>Oups!</h1>';

$('.demo').html(sanitizer.sanitize(theRawHTML));
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

<div class="demo"></div>

TLDR;

  • Avoid rendering HTML from untrusted sources, and find alternatives such as sending your inputs as JSON which you build the HTML for.
  • Rely on trusted and well-maintained libraries such as DOMPurify to sanitize your HTML
Fcmam5
  • 4,888
  • 1
  • 16
  • 33
  • 1
    `setHtml` is basically using the same `Sanitizer` API that is "not yet well supported", and it's even being developped, with a lot of open questions, so expect its behavior to change. – Kaiido Mar 10 '23 at 23:38
  • Thank you very much for your input, I will rephrase my answer – Fcmam5 Mar 11 '23 at 08:32
  • 1
    My point was more about `Element#setHTML` being at least as experimental as `Sanitizer` (actually even more). The MDN article is right that this is what should be used instead of `innerHTML`, but they're going a bit fast in recommending it today given that even at the specs level things aren't fixed yet. – Kaiido Mar 11 '23 at 10:24