Sanitizing global string attributes - necessary or not?

Question

Here is a DOM-based XSS flaw reported by SalesForce Security Review team in our code:

RootComponent

<aura:component extensible="true" abstract="true" access="global"
                description="Root template component">

   <!-- ... lot of other code ... -->

   <aura:attribute name="label" 
                   type="String" 
                   access="global" 
                   description="Label of the form input element" />

   <!-- ... lot of other code ... -->

</aura:component>

SubComponent

<aura:component access="global" extends="c:RootComponent" 
                description="Child extension of RootComponent">

   <!-- ... lot of other code ... -->

   <label>{!v.label}</label>

   <!-- ... lot of other code -->

</aura:component>

According to SFDC, "any app can extend SubComponent component and pass in as a label and trigger a DOM-based XSS attack." The solution would be to "sanitize" label global attribute to avoid such attack. What is meant by sanitization of global attribute?

I tried to reproduce the so-called security flaw , but as label attribute is of type "String" , when we send it HTML, it is displayed as a string, not as interpreted HTML ... so I don't see where the security flaw could be :/

<aura:application >
    <aura:handler name="init" value="{!this}" action="{!c.doInit}" />

    <c:SubComponent aura:id="TestXSS" label="DOM-based XSS attacks" />

</aura:application >

JS Controller

({
    doInit: function(component, event, helper) {
        var XssAttack = '<img src=x onerror=alert(1) />';
       component.find('TestXSS').set('v.label',XssAttack);
    }
})

Output

We have global attributes everywhere. Our components are packaged and meant to be extended by client implementation. Do we actually need to sanitize them?

Answer 1

If this is really a label as you describe, then this is a false positive. However you may have left something out of your description. If you could please send me a message with the details of the issue -- it's enough to get the name of the App if this is for the review -- then I can look into it.

Answer 2

There is a slight possibility for XSS since you are allowing the RootComponent to be extended .

One could throw a mark up like below

<aura:component access="global" extends="c:RootComponent" 
            description="Child extension of RootComponent">
 <!-- ... lof of other code ... --> 
   <label>{!v.label}</label>

 <aura:unescapedHtml value="{!v.label}"/>

 <!-- ... lof of other code -->

 </aura:component>

So once the client implementing it uses tags like aura:unescapedHtml they can easily escape it .

I was able to invoke the javascript using the below in the controller

  ({
      doInit: function(component, event, helper) {
       var XssAttack = '<a href=javascript:alert(1);> test </a>';
       component.find('TestXSS').set('v.label',XssAttack);
      }
   })

Sanitizing

Sanitize it with the help of the secure filters library here

There is a Trailhead module here on how to implement it

All you will need to do is make sure ultimately you wrap the global attributes that client can implement with secureFilter

Once you have SecureFilter in static resource use the below

<ltng:require scripts="{!$Resource.securefilters}" />

and sanitize the input using the html secure

({
   init: function(component, event, helper) {
    var label = component.get("v.label");
    label = secureFilters.html(label);
    component.set("v.label",label);
  }
})

Also you may add this with aura:change function to detect if label value changes and run this function .