[SalesForce] Checkmarx and JSON.serialize

My Salesforce1 code has been recently run through Force.com Security Scanner (a.k.a. Checkmarx). A stored XSS vulnerability has been found and marked as critical security risk.

It boils down to something like this:

// controller
public String getRecent(){
    return JSON.serialize([SELECT Id, Type, Name
        FROM RecentlyViewed
        WHERE Type IN ('Account', 'Contact')
        ORDER BY LastViewedDate DESC]);
}

// JS on VF page
var recent = {!recent};

Checkmarx recommends using JSENCODE, HTMLENCODE, URLENCODE etc to fix the vulnerability and shut the scanner up.

My question: really? I think it's a false positive. In what situation JSON.serialize would fail to properly escape quotes and / or html tags?

And if it's legit – how can I protect this case without screwing up the data to a point where parsing it back / unescaping becomes a pain?

I could fudge it by URLENCODING and decoding back I guess but I'd like to know if there's a real life scenario I should be worried about. After all 1 parse error and the relevant bit of my JS won't run.

Best Answer

Your page's javascript is effectively using the eval style of deserializing JSON which has been frowned upon in general ever since browsers started having pretty widespread support for JSON.parse.

If you don't need to support old versions of IE (and if you do there's polyfils) it's really, really a better idea to do something like

var recent = JSON.parse('{!JSENCODE(recent)}');

Is it really that much better from a security perspective? Not really dramatically in your case, but in more complex situations it can possibly have a tangible impact.

Also, since JSON is a subset of javascript in modern browsers it's not uncommon to see a performance boost using JSON methods over eval-style. JSON methods also handle escape characters that might be invalid in other places.

Related Topic