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