Hi Madhan,

A second order XSS (I.e. stored) can be executed against Atlas quite easily. If 
you create an entity with a name like

“><svg onload=“alert(‘Here I am’);” display=none>

You will get an alert when clicking “edit” and you will get artifacts all over 
the place stemming from the closing ‘“>’. And there are probably other areas 
where I can do this. Given the fact I can almost store arbitrary sizes into 
Atlas it does not take a lot of imagination to think of a rogue service adding 
an entity (or something else) with a malicious payload and then just wait for a 
user to pick it up.

This is not an “XSS in the UI”. OWASP states that “ DOM Based XSS (or as it is 
called in some texts, “type-0 XSS” or “CLIENT SIDE XSS”) is an XSS attack 
wherein the attack payload is executed as a result of modifying the DOM 
“environment” in the victim’s browser used by the original client side script, 
so that the client side code runs in an “unexpected” manner. That is, the page 
itself (the HTTP response that is) does not change, but the client side code 
contained in the page executes differently due to the malicious modifications 
that have occurred in the DOM environment. This is in contrast to other XSS 
attacks (stored or reflected), wherein the attack payload is placed in the 
response page (due to a server side flaw).

As shown above I was able to make the server return malicious code by storing 
data and have it returned at a different time. This makes it a Stored XSS. 
OWASP 
(https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html)
 states that to prevent stored XSS it should be addressed server side. 

Note also that the UI deals with escaping differently at different places. For 
example the “user labels” are being escaped (but not properly unescaped) on the 
client side. Entity definitions allow anything. Server side validation is not 
done, so I can easily circumvent the UI and edit the request.

I assume that you agree with the fact that bare HTML and JavaScript do not 
really have a place in Atlas definitions or a need? Why not escape data on 
output and make that configurable if really needed? And lets improve the CSP 
headers while we are at it (this is being looked at as you mentioned)?

Cheers
Bolke

Verstuurd vanaf mijn iPad

>> Op 11 jun. 2020 om 02:30 heeft Madhan Neethiraj <mad...@apache.org> het 
>> volgende geschreven:
> Melinda,
> 
> As I said earlier, I don't agree on server side escaping HTML characters in 
> its input and output. Escaping must be handled by components that are prone 
> to issues in dealing with unescaped data. We are discussing potential XSS in 
> UI while dealing with data from user/server, which requires UI updates to 
> perform necessary escapes. As I said earlier, Keval is already looking into 
> this.
> 
> And, I haven't heard yet (from you/Bolke/anyone else) on any server side 
> issues that require Atlas server to escape HTML characters in data it 
> receives/responds. I strongly suggest to not treat this as a release blocker. 
> If anyone thinks it is, please come out with clear use cases/examples quickly.
> 
> Regards,
> Madhan
> 
> 
> 
> On 6/10/20, 3:22 PM, "Melinda Crane" <mcr...@snap.com.INVALID> wrote:
> 
>    Dear Madhan and Bolke,
> 
> 
>    Are there any updates on this topic? The discussion so far has been pretty
>    exciting to see and hear - having the sanitization feature as an optional
>    config like Bolke mentioned would be real useful to us (since we know our
>    data should never contain html characters), and it seems like it would be
>    useful for other companies down the road who have stricter security
>    requirements. Where the sanitization would happen is of course completely
>    up to you folks!
> 
> 
>    Cheers,
> 
>    Melinda Crane
> 
>>    On Tue, Jun 9, 2020 at 10:46 AM Madhan Neethiraj <mad...@apache.org> 
>> wrote:
>> 
>> Bolke,
>> 
>>>    1. Filtering input on arrival where the user cannot manipulate it
>> anymore (i.e. server)
>> It's not clear what you expect the server to do here? Should HTML
>> characters be escaped in all inputs, before saving the data in its store?
>> Can you give few examples of server-side manipulation due to unescaped HTML
>> characters?
>> 
>>>    2. Encode data on output
>> I think asking the server side to escape all HTML characters in its
>> response is a very bad idea. Consider forcing such a requirement on a RDBMS
>> - wouldn't this mandate every client to un-escape every column value
>> returned for queries? Can you add references to applications that perform
>> HTML escape of all its REST API responses?.
>> 
>>>    3. Set correct Http content headers (e.g. application/json)
>> This is already being done. Do you find this missing for any specific
>> usecases?
>> 
>>>    4. Set correct Content Security Policy.
>> This is being looked into.
>> 
>> Regards,
>> Madhan
>> 
>> 
>> On 6/9/20, 12:49 AM, "Bolke de Bruin" <bdbr...@gmail.com> wrote:
>> 
>>    Hi Madhan,
>> 
>>    I don’t think solving this on the front-end is sufficient and the
>> defense should be more in-depth. The front-end is after all client side and
>> can be manipulated by the user. Typically one solves XSS vulnerabilities by:
>> 
>>    1. Filtering input on arrival where the user cannot manipulate it
>> anymore (i.e. server)
>>    2. Encode data on output
>>    3. Set correct Http content headers (e.g. application/json)
>>    4. Set correct Content Security Policy.
>> 
>>    Your proposed solution does not fit any of those criteria. If you are
>> worried about compatibility I suggest making it a server side configuration
>> variable (e.g. atlas.server.insecure_escaping) which should default to
>> “false” imho and alternative could be to enable configurable escaping but
>> that’s more complex.
>> 
>>    Note that in this case also #4 CSP headers seem to be very loose
>> (unsafe-inline, unsafe-eval) which add to the attack surface:
>> 
>> 
>> https://github.com/apache/atlas/blob/master/webapp/src/main/java/org/apache/atlas/web/filters/HeadersUtil.java#L44
>> 
>>    Regards,
>>    Bolke
>> 
>>    Verstuurd vanaf mijn iPad
>> 
>>> Op 9 jun. 2020 om 06:04 heeft Madhan Neethiraj <mad...@apache.org>
>> het volgende geschreven:
>>> Melinda,
>>> Thank you for adding details. I think the frontend must take steps
>> to prevent JavaScript execution while handling values in REST API responses.
>>> Server side escaping all HTML characters in all responses wouldn’t
>> be the right approach, as browser is not the only consumer of the
>> responses. Consider applications that parse response from Atlas REST APIs;
>> all such applications must be updated to deal with escaped HTML characters
>> in responses. This will likely break existing Atlas integrations.
>>> Hope this helps.
>>> Regards,
>>> Madhan
>>> On 6/8/20, 6:31 PM, "Melinda Crane" <mcr...@snap.com.INVALID>
>> wrote:
>>>   Apologies, looks like the second half of the email I just sent got
>>>   formatted weirdly (I was copying from a failed earlier email
>> attempt that
>>>   got rejected for attaching images). Just to make sure it doesn’t
>> get buried:
>>>   Of course this specific case can be patched on the frontend, but
>> doing some
>>>   kind of HTML sanitization escaping into the
>> whatever-is-handling-the-ajax
>>>   (EntityREST or AtlasEntityStoreV2, maybe?) to blanket-catch HTML
>> characters
>>>   sounds safer.
>>>   Hope this helps express my concerns.
>>>   From, Melinda Crane
>>>>   On Mon, Jun 8, 2020 at 9:13 PM Melinda Crane <mcr...@snap.com>
>> wrote:
>>>> Dear Madhan,
>>>> Thank you kindly for your response and for offering to look into
>> potential
>>>> solutions. I've been trying to step through how the ajax calls are
>> handled,
>>>> I don't really see the AtlasJsonProvider being used much at all
>> actually
>>>> for that. My concern is that the data from the ajax calls aren't
>> being
>>>> sanitized on the backend. I see there is a ton of sanitization
>> happening on
>>>> the frontend (lots of calls to _.escape()), but they're not all
>> captured.
>>>> For example.
>>>> In CommonViewFunction, in the propertyTable
>>>> <
>> https://github.sc-corp.net/Snapchat/keystone/blob/master/dashboardv2/public/js/utils/CommonViewFunction.js#L267>
>> function,
>>>> URLs are not escaped. So I can create an entity that has a key
>> value of
>>>> https://www.google.com/search?\";><iframe src='\''/'\''
>> width='\''1'\''
>>>> height='\''1'\'' onload='\''window.alert(\"boo\");'\''></iframe>
>>>> and my script will successfully get successfully injected into my
>> browser
>>>> when I try to view it. Looks like I can't attach images here but
>> here's the
>>>> elements:
>>>> <div class="entity-detail-table">
>>>>                   <table class="table">
>>>>                       <tbody data-id="detailValue"
>>>> class="hide-empty-value">
>>>>                       <tr class="hide-row">
>>>>                           <td>newSchema</td>
>>>>                           <td>
>>>>                               <div class="scroll-y">
>>>>                               N/A
>>>>                               </div>
>>>>                           </td>
>>>>                       </tr>
>>>>                       <tr class="">
>>>>                           <td>classField</td>
>>>>                           <td>
>>>>                               <div class="scroll-y">
>>>>                                   <a target="_blank"
>> class="blue-link"
>>>> href="https://www.google.com/search?";>
>>>>                                       <iframe src="/" width="1"
>>>> height="1" onload="window.alert(&quot;boo&quot;);"></iframe>
>>>>                                       "&gt;
>> https://www.google.com/search
>>>> ?"&gt;
>>>>                                       <iframe src="/" width="1"
>>>> height="1" onload="window.alert(&quot;boo&quot;);"></iframe>
>>>>                                  </a></div></td> ...
>>>> ^You can see the iframe-script combo here.
>>>> Of course this specific case can be patched on the frontend, but
>> doing
>>>> some kind of HTML sanitization escaping on the
>>>> whatever-is-handling-the-ajax (EntityREST or AtlasEntityStoreV2,
>> maybe?) to
>>>> blanket-catch HTML characters sounds safer.
>>>> Hope this helps express my concerns.
>>>> From, Melinda Crane
>>>>> On Sat, Jun 6, 2020 at 2:48 PM Madhan Neethiraj <mad...@apache.org>
>> wrote:
>>>>> Melinda,
>>>>> Thank you for reaching out to Apache Atlas community.
>>>>> As you noted, AtlasJsonProvider is used to deserialize/serialize
>> REST API
>>>>> requests and responses. In addition, methods in AtlasJson are used
>> in to
>>>>> convert to/from Json. It will help if you can add few examples of
>> potential
>>>>> issues with the current implementation.
>>>>> Keval is looking into the frontend for potential issues and should
>> get
>>>>> back early next week.
>>>>> Thanks,
>>>>> Madhan
>>>>> On 5/29/20, 6:50 PM, "Melinda Crane" <mcr...@snap.com.INVALID>
>> wrote:
>>>>>   Dear Apache Atlas devs,
>>>>>   I’m from Snapchat, we’re working on building a catalog on top of
>>>>> Apache
>>>>>   Atlas. I noticed looking at the frontend for Atlas that there
>> seem to
>>>>> be
>>>>>   several places vulnerable to XSS, so I’ve got some questions
>> for the
>>>>>   community (especially frontend folks) or other partner
>> companies who
>>>>> may
>>>>>   have dealt with similar problems:
>>>>>      1.
>>>>>      the CSP allows unsafe-inline and unsafe-eval
>>>>>      2.
>>>>>      the backend JSON content provider, AtlasJsonProvider, doesn’t
>>>>> appear to
>>>>>      do any forced escaping.
>>>>>   Regarding the unsafe-inline and unsafe-eval, I saw some inline
>>>>> scripting
>>>>>   going on with the index html file in dashboardv2/v3, but that
>> doesn’t
>>>>> look
>>>>>   bad to move out. However, I noticed a lot of the third party
>> node
>>>>> modules
>>>>>   use inline scripting (such as backgrid-filter and
>>>>>   bootstrap-daterangepicker) and/or eval() (such as
>>>>> javascript-stringify and
>>>>>   even requirejs). I’d like to get rid of unsafe-inline and
>> unsafe-eval
>>>>> in my
>>>>>   CSP; are there any recommendations on how to deal with the
>> third party
>>>>>   plugins to achieve this?
>>>>>   For the forced escaping, are there any other major places on
>> backend
>>>>>   besides the JSON provider that should definitely be escaped?
>> Also,
>>>>> since
>>>>>   the AtlasJsonProvider seems to be *the* default object mapper,
>> are
>>>>> there
>>>>>   any insidious or otherwise consequences of force escaping HTML
>>>>> sensitive
>>>>>   characters in it?
>>>>>   Thank you kindly for any advice.
>>>>>   From, Melinda Crane

Reply via email to