Take the 2-minute tour ×
Salesforce Stack Exchange is a question and answer site for Salesforce administrators, implementation experts, developers and anybody in-between. It's 100% free, no registration required.

The CheckMarx security scanner says that the 4th line:

$j('#h{!category}').on('click', function(event) {

in the following script, causes an XSS vulnerability. Can someone explain the best way to fix it? or if it's a false positive, how can I rewrite the script so it does not happen?

<script type="text/javascript" charset="UTF-8">
        $j = jQuery.noConflict();
        $j(document).ready(function(){
        $j('#h{!category}').on('click', function(event) {        
             $j('#s{!category}').toggle();
             $j('#h{!category}').css("background-color","{!entity.CPHeader__c}");
             $j('#lmenu').css("width","195px")
             $j('#mainbodyout').css("left","220px")
        });
            $j('#lmenu').on('mouseout', function(event) {        
                $j('#s{!category}').hide();
            });
            $j('#s{!category}').on('mouseover', function(event) {        
                 $j('#s{!category}').show();
            });
        });
    </script>

Thanks!

share|improve this question

2 Answers 2

up vote 4 down vote accepted

I believe its because you are using an unescaped output in your JS

$j('#h{!category}').css("background-color","{!entity.CPHeader__c}");

. Instead use the JSENCODE formula

$j('#h{!category}').css("background-color","{!JSENCODE(entity.CPHeader__c)}");

for further details see
https://developer.salesforce.com/page/Secure_Coding_Cross_Site_Scripting#S-Control_Template_and_Formula_Tags

share|improve this answer

The Checkmarx scanner is flagging "naked" (e.g. unencoded) merge-fields in a javascript context, so the following will quiet the scanner:

<script type="text/javascript" charset="UTF-8">
    $j = jQuery.noConflict();
    $j(document).ready(function(){
    $j('#h{!JSENCODE(category)}').on('click', function(event) {        
         $j('#s{!JSENCODE(category)}').toggle();
         $j('#h{!JSENCODE(category)}').css("background-color","{!JSENCODE(entity.CPHeader__c)}");
         $j('#lmenu').css("width","195px")
         $j('#mainbodyout').css("left","220px")
    });
        $j('#lmenu').on('mouseout', function(event) {        
            $j('#s{!JSENCODE(category)}').hide();
        });
        $j('#s{!JSENCODE(category)}').on('mouseover', function(event) {        
             $j('#s{!JSENCODE(category)}').show();
        });
    });
</script>

You may want to assign the merge-fields to variables first or use an anonymous function:

<script>
var category = "{!JSENCODE(category)}";
[...]
$J('#h' + category).on(...)
[...]

or

</script>
(function(c, h) {   [...]     }  
 )('{!JSENCODE(category)}', '{!JSENCODE(entity.CPHeafder__c)}')
</script>

As to whether this is a false positive, it depends on what the values of the merge-fields can be.

However, as a best practice, you should protect these fields with JSENCODE anyways, otherwise you are creating an unnecessary coupling between your controller and javascript code. For example now assume that category is an enum. In the future, you might make the code more dynamic and pull a value from the db. Suddenly you have introduced a stored XSS into your page without changing any of your page code.

This will also make your code easier to audit because you won't need to track down the possible values of 'category' when determining whether this page is vulnerable or not.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.