Hi, Offering my two cents. I'm a security engineer at Snap working with Melinda to review her service which uses Atlas. While the XSS found here can be hotfixed, it doesn't provide confidence that it won't occur elsewhere without a broader solution.
For fixes, I don't think it's unreasonable to filter user inputs server-side, especially when we know they're rendered as HTML. Alternatively, on the frontend it would be better if we could sanitize at a central point, rather than trying to escape fields as they're used. If there isn't a central point, then somewhere closer to where JSON responses are fetched and serialized would also work. The library in its current state would fail to get internal approval for production usage. XSS like this has been addressed in other frameworks for years so there's little reason for a modern webapp to still have them. If these features aren't fit for the public release we'll likely find a different solution or try to add them internally, but they might be less elegant than if they were developed by the maintainers. Justin On Mon, Jun 15, 2020 at 3:34 PM Melinda Crane <mcr...@snap.com> wrote: > > > ---------- Forwarded message --------- > From: Madhan Neethiraj <mad...@apache.org> > Date: Mon, Jun 15, 2020 at 6:12 PM > Subject: Re: Seeking advice on Atlas XSS vulnerabilities > To: dev@atlas.apache.org <dev@atlas.apache.org>, Bolke de Bruin < > bdbr...@gmail.com>, Melinda Crane <mcr...@snap.com> > > > Bolke, Melinda, > > I think it will help to clarify what I meant by "server-side" and "UI". > - server-side: Atlas REST APIs to perform various operations on > types/entities/glossaries/.. > - UI: a web-application running in a browser which calls Atlas REST APIs > to fetch data, render and update data > > I completely agree on all UI modules to handle HTML escaping appropriately > - irrespective of whether they run in browser or server side. Few > references to 'server side' in the documents referred in this thread seem > to be about servlets/JSP - which are not used in Apache Atlas. > > If you are suggesting all Atlas REST API responses to escape HTML - I > think it is a very bad idea. BTW, none of the documents referred in this > thread ask the server REST APIs to perform any escaping for HTML. > > Regards, > Madhan > > On 6/15/20, 1:48 PM, "Bolke de Bruin" <bdbr...@gmail.com> wrote: > > Hi Keval > > I'm happy that you will tackle the UI part of it. However, these are > not mere UI issues and need to be tackled server side IMHO. This opinion is > supported by what the experts say[1] > > Can you elaborate on how you will approach this? I have not seen a > discussion. > > Cheers > Bolke > > [1] > https://urldefense.proofpoint.com/v2/url?u=https-3A__security.stackexchange.com_questions_19524_current-2Dbest-2Dpractices-2Dto-2Dprevent-2Dpersistent-2Dxss-2Dattacks_19536&d=DwIFaQ&c=ncDTmphkJTvjIDPh0hpF_w&r=IkBo18xErS3TGsJ4f2MH4HIFKDcWTAyKWqDieefibV4&m=W6dQ86TlCtMFcgDcupU4BuWnH1Oh2gxXD7_Y4AUuztA&s=8OVEzsh4MotsUL__aDfRl_bkm6THou4bBPMP2hDYhOM&e= > > > Sent from my iPhone > > > On 15 Jun 2020, at 19:47, Keval Bhatt <kbh...@apache.org> wrote: > > > > Hi Bolke, > > > > Thanks for the details on the UI issues in rendering entity name and > > incorrect value storied for user-defined attributes. I am working on > > updating UI to address these issues, will send a patch shortly. > > > > If you see further issues that require a server-side update, can you > please > > add specific use case details? > > > > Thanks > > > >> On Fri, Jun 12, 2020 at 2:15 PM Bolke de Bruin <bdbr...@gmail.com> > wrote: > >> > >> 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://urldefense.proofpoint.com/v2/url?u=https-3A__cheatsheetseries.owasp.org_cheatsheets_Cross-5FSite-5FScripting-5FPrevention-5FCheat-5FSheet.html&d=DwIFaQ&c=ncDTmphkJTvjIDPh0hpF_w&r=IkBo18xErS3TGsJ4f2MH4HIFKDcWTAyKWqDieefibV4&m=W6dQ86TlCtMFcgDcupU4BuWnH1Oh2gxXD7_Y4AUuztA&s=oPgkBmLIj_UFLzOBRIo11L-VRSeSpzcwz5RqlbBIwJU&e= > ) > >> 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?%5C";><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("boo");"></iframe> > >>>>>> "> > >>>> https://www.google.com/search > >>>>>> ?"> > >>>>>> <iframe src="/" width="1" > >>>>>> height="1" onload="window.alert("boo");"></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 > >> > > >
