Stored XSS in database.query

Question

We have submitted our code to security review. i'm facing stored xss vulnarabities as described in below lines, please some one help me on this.

 public map<string,RecordType> RecordTypeMap(String ObjectName){                    
     map<string,RecordType> recdmap = new map<string,RecordType>();
     if(ObjectName!=''){
       for(RecordType RT :[select id,Name,SobjectType,NamespacePrefix,IsActive,DeveloperName,Description,BusinessProcessId from RecordType where SobjectType=:ObjectName ]){
         recdmap.put(String.ValueOf(RT.id),RT);      
       }    
     }
     return recdmap;
}

public Map<string,Calendar_Settings__c> CustomSettForSobject(string objectname){          
      map<string,Calendar_Settings__c> MapCustomSett = Calendar_Settings__c.getall();
      map<string,Calendar_Settings__c> MapRecTypeAndCSett = new map<string,Calendar_Settings__c>();
      for(Calendar_Settings__c sett :MapCustomSett.values() ){
            if(sett.objectName__c==objectname){                    
               MapRecTypeAndCSett.put(sett.RecordTypeName__c,sett);                    
            }
      }
      return MapRecTypeAndCSett;
}

 map<string,RecordType> rtyp = RecordTypeMap(SelectedObj);

Map<string,Calendar_Settings__c> MapRecTypeAndCSett = CustomSettForSobject(SelectedObj);
 //checking already FLS in a util class before going to going to execute this query 

   if(MapRecTypeAndCSett!=null && rtyp!=null){               
      for(Calendar_Settings__c clndr : MapRecTypeAndCSett.values() ){
        for(RecordType rt :rtyp.values()){
          if(RT.SobjectType == clndr.objectName__c){                           
            if(clndr.RecordTypeName__c == RT.name ){                  
               recordtypeandmap.put(String.ValueOf(RT.Id),RT.name);      
            }              
            if(clndr.RecordTypeName__c == RT.developername){              
                recordtypeandmap.put(String.ValueOf(RT.Id),RT.developername);                      
            }
          }
        }
      }

    }

    L 195: for(sObject e: Database.query(eventsQuery)){ 


L 307: if(e.get('RecordTypeId')!=''){ ... }
L 308: if(recordtypeandmap.ContainsKey((string)e.get('RecordTypeId'))){ .. }
L 317: if(e.get(MapRecTypeAndCSett.get(recordtypeandmap.get((string)e.get('RecordTypeId'))).EndDateField__c) !=null)&&(...){...
     ClndrEvent cE = new ClndrEvent();
     cE.allDay = false;
     cE.url = e.Id;
    cE.timezoneParam = tz; 
    cE.sobj = string.ValueOF(e.getSObjectType());
  cE.rectype = recordtypeandmap.get((string)e.get('RecordTypeId'));             
  cE.title =(string) e.get( 
 MapRecTypeAndCSett.get(recordtypeandmap.get((string)e.get('RecordTypeId'))).subject__c); 
    }
        L 504: JSONString = JSON.serialize(eventsList);
        L 506: JSONString = JSONString.replaceAll('pEnd', 'end');
                      ... 
            }

Visualforce ....

these are the few regions that i suspect in page,







function reloadCal() {

    var selectedItems = [];
    var check = document.getElementsByName("fltr");
    var fltrstring = '';
    for (var i=0; i<check.length; i++) 
     {

       if(check[i].checked==true){
        selectedItems.push(check[i].value);
        fltrstring = fltrstring+check[i].value+',';
    }
    }
    var myval='';
    $("input:radio").each(function () {


var $this = $(this);


if (this.checked) {
    myval = this.value;
    //alert(myval);

}

})

    console.log(selectedItems);
    console.log('--fltrstring--'+fltrstring );
    filterallevents(fltrstring,myval);

}




    function reloadevntsandtask(){
    console.log('Hello');
    console.log('-Is nOt Updated from Comp--'+'{!IsNotUpdated}');
    var localvar = '{!IsNotUpdated}';
    if(localvar=='false'){
    console.log('-Is Within the false --'+'{!IsNotUpdated}');
    reloadCal();
    }
    else {
    return false;
    }

    }


</script>
<apex:outputPanel id="rlde">
    <script type="text/javascript" language="javascript">
    function locadclnr() {

        $('#calendar').fullCalendar({
           height:625,
            header: {
                left: 'month,agendaWeek,agendaDay',
                center: 'title',
                right: 'prevYear prev,today,next nextYear'
            },
            defaultView: 'month',
            allDaySlot:true,
            eventLimit: true,
            theme: true,
            events: {!JSENCODE({!Events})} ,
            eventRender: function(event, element) {
                element.attr('href', 'javascript:void(0);');
                element.click(function() {


                    fetchevent(event.url,event.sobj);

                    console.log(event.url);




                });




            },


        }); 



    }

    locadclnr();
    </script>
</apex:outputPanel>


    <apex:actionFunction status="ajaxldr" name="fetchevent" action="{!editevent}" reRender="frm">
    <apex:param value="" name="evntid" />
    <apex:param value="" name="sfobj" />
</apex:actionFunction>
<apex:actionFunction status="ajaxldr" name="filterallevents" action="{!filterevents}" reRender="clndrpnl,frm,rlde,">
    <apex:param value="" name="filterby" />
    <apex:param value="" name="changedview" assignto="{!viewtype}" />
</apex:actionFunction>
<apex:actionStatus id="ajaxldr" onstart="blockoverlay();" onstop="$.unblockUI();" />

          <div style="margin-right:30px;"  >  <!-- border:1px outset lightgrey;border-radius:5px; -->
            <legend style="color:white;font-weight:150;margin-bottom:0px;font-size:16px;border-radius:4px;background:{!headerColorvalue}  ;"><span style="padding-left:6px;">Event Information</span>
            </legend>
            <br/>
            <div align="center">
                <apex:commandButton status="ajaxldr" action="{!ToggleEditViewOfEvent}" title="Edit" reRender="frm,rlde" value="Edit" styleclass="btnedit" />
            </div><br/>
            <apex:pageblocksection columns="1" >
                    <apex:outputField value="{!evt.Subject}" styleclass="formcontrol1" />
                   <apex:outputField value="{!evt.activityDateTime}" styleclass="formcontrol1" />
                    <apex:outputField value="{!evt.StartDateTime}" styleclass="formcontrol1" />
                    <apex:outputField value="{!evt.EndDateTime}" styleclass="formcontrol1" />
                    <apex:outputField value="{!evt.Type}" styleclass="formcontrol1" />
                    <apex:outputField value="{!evt.DurationInMinutes}" styleclass="formcontrol1" />
                    <apex:outputField value="{!evt.Description}" styleclass="formcontrol1" />
            </apex:pageblocksection>
            </div>


                                <apex:repeat value="{!CalendarSettings}" var="filter">
                    <div id="inputcheckbox" class="inputcheckbox" style="display: inline-block;">
                        <label style="font-weight:normal;">
                            <span style="text-align:right; margin-left:10px;cursor:pointer;" onmouseover="this.style.textDecoration='underline';" onmouseout="this.style.textDecoration='none';this.style.color='black';">

            <input type="Checkbox" id="slideThree" onchange="reloadCal();" class="rectypefilter" name="fltr" value="{!filter.RecordTypeName__c}" style="margin-right:3px;">{!filter.name}</input> 

            </span></label>
                    </div>
                </apex:repeat>



                            <div id="cal-legend">
            <ul>
                <li style="border:1px solid lightgrey">
                    <apex:commandLink id="clEvent" onclick="changeColor('{!$Component.clEvent}', '{!headerColorvalue}  '); changeColor('{!$Component.clTask}', 'white');" status="ajaxldr" action="{!ToggleCalendar}" reRender="clndrpnl,frm,rlde,fltrepeat"  onmouseout="this.style.textDecoration='initial';this.style.color='#337ab7';" onmouseover="this.style.textDecoration='underline';" style="text-decoration: blink;font-weight: bold;" value="Events">
                        <apex:param name="evnt" value="{!EvntSel}" assignTo="{!SelectedObj}" /><span style="margin-top:4px;"><i class="fa fa-th"></i></span>
                    </apex:commandLink>
                    </li>

                <li style="border:1px solid lightgrey">
                    <apex:commandLink id="clTask"  onclick="changeColor('{!$Component.clEvent}', 'white'); changeColor('{!$Component.clTask}', '{!headerColorvalue}  ');" status="ajaxldr" action="{!ToggleCalendar}" reRender="clndrpnl,frm,rlde,fltrepeat" onmouseout="this.style.textDecoration='initial';this.style.color='#337ab7';" onmouseover="this.style.textDecoration='underline';" style="text-decoration: blink;font-weight: bold;" value="Tasks">
                        <apex:param name="evnt" value="{!TaskSel}" assignTo="{!SelectedObj}" /><span style="margin-top:4px;"><i class="fa fa-tasks"></i></span>
                    </apex:commandLink>
                    </li>
            </ul>
            <div style="clear:both;">
                <!--fix floats-->
            </div>
        </div> 



            .panel-primary>.panel-heading {
color: #fff;
background-color: {!headerColorvalue};
border-color: grey;
}

//keep the shape of the boxes we're hiding
.fc-other-month {
    visibility: hidden
}
.fc-toolbar .fc-state-active, .fc-toolbar .ui-state-active{
background:{!subheaderColorvalue} ;
}
.fc-toolbar button {
position: relative;
background: {!headerColorvalue}  ;
color: white;
border-radius:5px;
}

body button:hover, body .btn:hover, body .btnCancel:hover, body .menuButton .menuButtonButton:hover{
background:{!subheaderColorvalue} ;
}

.fc th {
 padding:0px;
 background:{!headerColorvalue}  ;
 color:white;
}
</style>

Answer 1

OK, I see a couple of incorrect encoding issues that might lead to XSS. Whether or not they are exploitable or false positives is going to depend on the data type -- you only need to encode strings, not booleans or integers, but I can't determine the datatype in all cases from looking at the code. If this is an automated scan, then the scanner probably also is not able to reason accurately about the data type.

Here are the issues:

Any time you have a mergefield in a <script> or <style> context, the auto-html encoding is disabled and you have the possibility of script injection. E.g.:

ex. 1: background:{!subheaderColorvalue} ; //style context
ex. 2: console.log('-Is nOt Updated from Comp--'+'{!IsNotUpdated}'); //script context
ex. 3: events: {!JSENCODE({!Events})} , //script context
ex. 4: onclick="changeColor('{!$Component.clEvent}', 'white'); changeColor('{!$Component.clTask}', '{!headerColorvalue}  ');" //js event handler context

There may also be other issue types (e.g. DOM based XSS), as you've only shown a portion of the code.

So what is the attack and how to properly sanitize?

For example1, the attack would be to close out the style and write your own code. No escaping at all is done in style tags. This assumes your mergefield is a string ("blue") rather than a hex number. But be careful because even though you may intend something to be a number, if it is pulled from a URL parameter and not cast to a number, then it's just a string.

Example 1 is hard to sanitize, and we really discourage the use of mergefields in style contexts. Better to define your styles and then make dynamic style changes in JS rather than in the style tag itself. But if you must, your best bet is HTMLENCODE(). That does not catch all possible issues, though.

For example 2: If this is a string, you need to JSENCODE(). If it's a boolean, then it's not an issue except again you need to force it to be a boolean rather than just expect a boolean or assume a boolean.

For example 3: I see you are trying to JSENCODE but this does nothing for you. The purpose of JSENCODE is to prevent breaking out of quotes. It's nothing more than the JS version of StringEscapeSingleQuotes. But if your value isn't quoted to begin with, it's a null-op. So you need to add quotes. The reason you didn't add quotes is because you wanted JSON -- but you can't have it. You cannot safely pass complex types to the page via server-side rendering, you must serialize those types. To get direct access to complex types use xhr (JS Remoting) or equivalent. Server side rendering always produces a string. So to fix, change to:

JSON.parse('{!JSENCODE(strEvents)}')

where strEvents is the stringified version of events, stringified on the server in Apex. I suspect this is the primary architectural flaw in your code. Never try to pass complex types to the page via server-side rendering technologies such as merge-fields. Templating expressions are not capable of doing that.

For example 4:, the platform will auto-html encode the mergefield but that's not enough, because it is also passed to a JS execution context after html decoding. So here you need to also JSENCODE any types not cast to integers or booleans. E.g. '{!JSENCODE(headerColorvalue)} ');" To see what can happen if you don't do that, suppose that headerColorvalue was:

0');alert(document.origin);//

When it is rendered on the page, because it is not in a script or style context, it will be html encoded:

0');alert(document.origin);//

So you will not be able to break out of the onclick event handler. Unfortunately, you don't need to break out of this handler to execute code, because this mergefield is already evaluated in a JS execution context. You only need to break out of the string literal variable expression within the changeColor function to inject your own JS code. To do that, you need to only insert a single quote.

Moreover, what is passed to the script engine when the event handler is invoked is the html decoded version of the event handler:

0');alert(document.origin);//

and this will succeed in breaking out of the variable declaration. So to prevent that single quote from creating a breakout, you need to JSENCODE. Again, this is assuming that the value is a string and not an integer.

There may also be other issues, but these are the exemplar issues I've seen in the extracts you provided. Please also make sure that the user does not have full control over any hrefs.

For more information, please book an office hour.