cordova-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [cordova-plugin-whitelist] schmich edited a comment on issue #49: allow iframe without allow-navigation="*" breaking app security & links
Date Wed, 22 Jul 2020 08:32:49 GMT

schmich edited a comment on issue #49:
URL: https://github.com/apache/cordova-plugin-whitelist/issues/49#issuecomment-662237307


   Has there been any consideration given to this feature? We recently upgraded from `cordova-ios`
5.1.1 to 6.1.0, and this has become a problem for us on iOS.
   
   ### Details
   
   - Our Cordova app loads our site from `https://www.formulate.co/app`
   - Our site uses [Google Tag Manager](https://developers.google.com/tag-manager/quickstart)
for analytics and remarketing, so we include the GTM JS snippet on that page
   - GTM inserts an invisible `<iframe>` element that loads a page over HTTPS from the
`bid.g.doubleclick.net` domain
   - Our page has a lax CSP setting, so that is not interfering here
   - Our app uses the `WKWebView` engine for iOS, either via the `cordova-plugin-wkwebview-engine`
plugin (5.1.1) or through native support (6.1.0)
   - Our `config.xml` contains the following relevant settings:
   ```
   <access origin="*" />
   <allow-navigation href="*://*.formulate.co/app" />
   <allow-navigation href="*://*.formulate.co/app/*" />
   ```
   
   With **cordova-ios 5.1.1**, the iframe request was just silently rejected. I didn't actually
realize this was happening before, but with this issue, I now see that this is happening.
   
   With **cordova-ios 6.1.0**, the iframe request kicks the user out of the app into a new
Safari browser session that loads the `bid.g.doubleclick.net` URL. This is obviously a terrible
and unintended experience.
   
   The 6.1.0 experience actually seems more consistent with how I would expect Cordova to
behave, i.e. any URLs not matching an `allow-navigation` pattern are opened in a new browser.
Unfortunately, though, this leaves us with three options that seem to all have downsides:
   
   ### Option 1
   
   We stay with `cordova-ios` 5.1.1 or we upgrade but determine how to reject iframe requests
that don't match our `allow-navigation` patterns to emulate 5.1.1 behavior.
   
   Not upgrading is a problem since it's not a real, long-term solution. Alternatively, we
could configure the app to use 6.1.0 but behave like it did in 5.1.1 by rejecting iframe requests
not allowed by `allow-navigation`. However, I imagine this would defeat whatever the iframe
was trying to do and would interfere with analytics as I imagine it currently does in our
5.1.1 app.
   
   ### Option 2
   
   We add `<allow-navigation href="*" />` to our `config.xml`. Allowing navigation for
anything fixes the issue in 6.1.0. The iframe request succeeds transparently in the background
and the user remains in the app.
   
   The issue, of course, is that any top-level navigation is also allowed. Our app has package
tracking links for Fedex, UPS, etc. that we _want_ to load in an external browser. Choosing
this option breaks that scenario.
   
   ### Option 3
   
   We add `<allow-navigation href="*://bid.g.doubleclick.net/*" />` to our `config.xml`.
Similar to Option 2, this fixes the issue in 6.1.0 while also not breaking the top-level FedEx/UPS
package tracking links.
   
   There are two problems here:
   
   1. This allows top-level navigation to `bid.g.doubleclick.net` which we never want
   2. If they change the iframe URL or if any of our other imported dependencies (FB/IG/Snapchat/Pinterest
ads, other libraries) adds an iframe element of their own, we regress back to our original
problem of the iframe URL abruptly loading in a new browser session outside of the app
   
   These problems are less likely, and this does seem like the best option at the moment,
but it requires us to carefully document and track all possible iframe URLs that could ever
load. Additionally, since this is hard-coded in `config.xml`, any fix would require a full
app update with potential review which could take time. This option feels brittle.
   
   ### Proposal
   
   Based on what I've seen, it seems like the best solution would be to have separate `allow-navigation`
lists based on some idea of context like `app` (top-level), `iframe`, and potentially others
(I haven't thought much deeper about it).
   
   I'm not concerned with how this would be expressed, but as an example in this case, our
`config.xml` might look like the following:
   
   ```
   <allow-navigation context="*" href="*://*.formulate.co/app" />
   <allow-navigation context="*" href="*://*.formulate.co/app/*" />
   <allow-navigation context="iframe" href="*" />
   ```
   
   This would only allow specific URLs for top-level navigation while still allowing arbitrary
iframe navigation.
   
   ### Questions
   
   - Is there a way to do this already? Am I misunderstanding anything?
   - Otherwise, is there any chance of this feature happening?
   
   ---
   
   ### Update
   
   I found the following two related Jira issues from around 2017 that seem to have stalled
out:
   
   - https://issues.apache.org/jira/browse/CB-10709
   - https://issues.apache.org/jira/browse/CB-12455
   
   This doesn't look promising. Are there any pointers for working around this issue with
a plugin? Are there any pointers for digging into the `cordova-ios` source to fix this and
submit a PR?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


Mime
View raw message