4

We have a hybrid webforms/asp.net application which does a lot of partial-page updates from javascript using jquery.

The typical (unsafe) pattern in our application's javascript is to respond to a user request to re-write part of the page with something like this:

$.ajax({
        type: "GET",
        url: urlVariableHere,
        success: function (data) {
            $("#elementIdHere").html(data); 
        },
        error: function (XMLHttpRequest, ajaxOptions, ex) {
            errorHandlerFunction(XMLHttpRequest);  
        }

"urlVariableHere" points to an MVC Controller method that returns a rendered MVC view. In other words, the Controller method returns a blob of raw HTML.

This pattern is unsafe because of the call to JQuery's html() method, which is vulnerable to a cross-site scripting attack. We now need this application to pass a Veracode static analysis, and this unsafe pattern is repeated several hundred times.

Hooman pointed out in his answer that if we are calling a Controller method which renders a View which does not use the Html.Raw method we are safe from an XSS attack. The problem is, we need to pass a Veracode static scan, and for internal reasons we cannot mark these flaws as "mitigated." For internal reasons the application must pass a static scan with zero mitigations.

What is the best (i.e. most time-economical) way to make this application safe, and still keep the ability to do partial-page updates from javascript? Right now I only see three alternatives, all of them huge efforts:

  1. Change every partial-page postback to a full-page postback.
  2. Change every ajax call to fetch JSON instead of HTML, and then safely create DOM elements from the JSON using safe methods like document.createElement(), element.setAttribute(), element.appendChild() and etc.
  3. Re-write the application to use a javascript framework (Angular, Vue) or library (React).

Am I missing an easier solution?

Tom Regan
  • 3,580
  • 4
  • 42
  • 71
  • I am facing the same problems now with Veracode static scan. Which solution did you choose to implement? Any advice? – hdz200 May 09 '19 at 13:38
  • @Lion200 afraid I've not decided on one yet. – Tom Regan May 09 '19 at 17:18
  • Did you find a solution/workaround for this @TomRegan? We have the exact same problem with our SAST-scanning finding this flaw. – hightech Mar 03 '23 at 08:36
  • @hightech no, I don't think there is a solution other then a rewrite. We're doing new development in React and temporarily ignoring the security scan results. – Tom Regan Mar 03 '23 at 16:39
  • Okey, I thought so too.. Well, thank you anyways! – hightech Mar 04 '23 at 19:22
  • 1
    One alternative you didn't consider yet is to approach the Veracode support team and explain the fix you have developed. Maybe you will find a solution with them to suppress the false alert. You asked the question 4 years ago, and started a bounty recently - is there still no solution available? – Matt Mar 09 '23 at 07:16
  • @Matt this is not an issue with Veracode. Veracode is correctly noting that the jquery html() call is unsafe. Veracode allows entities that use its service to mark these issues as "mitigated." Our organization, internally, decided that it would not allow these issues to be marked "mitigated." I explained to the powers that be that this will require a re-write. – Tom Regan Mar 09 '23 at 13:10
  • 1
    Is there some reason you can't just update your jquery to use vanilla `innerHTML`? The `innerHTML` method by default won't evaluate or execute anything in a ` – Josh Mar 09 '23 at 20:46
  • @Josh that might work. You're suggesting that I alter the original post to something like this: document.getElementById("elementIdHere").innerHTML=data; – Tom Regan Mar 10 '23 at 13:39
  • @TomRegan - Thanks for the clarification. I thought you were getting an false alert from Veracode after applying the fix. – Matt Mar 13 '23 at 07:20
  • @TomRegan, yes that's pretty much what I'm suggesting. You may also want to take the opportunity to re-write whatever portions of the frontend code into modern vanilla JS/TS. There are still some security gotchas with `innerHTML`, but you should be just fine if your HTML is coming from trusted sources (i.e. not user input). I don't see any good reason to be running jQuery these days unless it's in a legacy project. – Josh Mar 13 '23 at 13:51
  • @Josh next time I do a Veracode test (likely be a couple of months) I'll change some of the calls to use inner HTML and see if it passes muster, and then update this question with the result. – Tom Regan Mar 13 '23 at 21:55
  • @hightech if you run another test soon please try using innerHTML as Josh suggested and let us know if it fixes the issue. – Tom Regan Mar 13 '23 at 21:56
  • @TomRegan - If `innerHTML` doesn't work for you, the other alternative would be to use a sanitizer. The JS `Sanitizer()` API is still experimental and not quite yet ready for prime-time, but you could potentially use a third-party library like `dompurify`. – Josh Mar 15 '23 at 14:12

1 Answers1

0

As far as I know, XSS is a problem when you are getting some input from the user, it is not clear to me why you should not trust the response from your own controller? What you are doing is very typical and I have seen countless number of tutorials teaching it (like c-sharpcorner, aspsnippets or dotnetthoughts, etc).

Also, ASP.NET MVC View Engine encodes HTML by default. I am not sure how you render your Partial View, but unless you are using @Html.Raw you will potentially double encode the result.

But if you want to encode the HTML result, you can escape your HTML string, see this answer

Hooman Bahreini
  • 14,480
  • 11
  • 70
  • 137
  • Thanks Hooman, but I'm afraid you missed the point. I trust the response from my own Controller. The issue is that the call to jQuery.html() fails a Veracode static scan, and for internal reasons we cannot mark these flaws as "mitigated." I need to render html, so I cannot use the jQuery text() method. – Tom Regan Jan 20 '19 at 17:17
  • @TomRegan, I have updated my answer. Would be nice if you could share what sort of error/advice you are getting from Veracode... – Hooman Bahreini Jan 20 '19 at 22:56
  • every use of html() is flagged as a medium flaw by Veracode. The particulars are not relevant to this question. I'm looking for advice on the easiest way to refactor. I know how the asp.net view engine works. – Tom Regan Jan 21 '19 at 13:45