Expose the setSupportMultipleWindows in Android to fix CVE-2020-6506

[mrcoinbase]

This PR exposes the setSupportMultipleWindows setting in Android to fix CVE-2020-6506. Let me know if there is anything missing from this PR. I suspect this may cause strange behavior in applications that assumed that this would be false by default.

[cristianoccazinsp]

Do we also need to set it to false to mitigate the XSS vulnerability?

[mrcoinbase]

Thanks for checking @cristianoccazinsp . By default, if it is true it will mitigate the vulnerability.

I'm following the similar fix that was used in google flutter https://github.com/flutter/plugins/pull/2991/files#diff-43c39528992b7b93988dcd7d78478281R129

[jamonholmgren]

Thank you @mrcoinbase! I believe if they set supportMultipleWindows to true, that we would also need a onCreateWindow method implementation, so this isn't quite complete. Do you have an idea of how we would handle that?

(For reference, this is how Flutter did it: https://github.com/flutter/plugins/pull/2991/files#diff-43c39528992b7b93988dcd7d78478281R77)

[jamonholmgren]

Could you mention the vulnerability if they set this to false?

[mrcoinbase]

Sure, good idea!

[mrcoinbase]

@jamonholmgren This appears to be the place we would add oncreatewindow. I'm not sure why flutter added it in their PR. https://github.com/flutter/plugins/pull/2991/files#diff-43c39528992b7b93988dcd7d78478281R96-R103 this is apparently to check if the url is secure. I'm not sure if we need to implement and override the method here:

react-native-webview/example/android/app/src/main/java/com/example/MainApplication.java

Line 47 in ac4e05e

@Override

Then again, this is just some guest development. If you believe we need to override that method and make changes, let me know.

[mrcoinbase]

Also @jamonholmgren, it seems that oncreate is used to make it vulnerable, but doesn't need to be modified to protect it, see 'How to identify vulnerable apps' in https://alesandroortiz.com/articles/uxss-android-webview-cve-2020-6506

[jamonholmgren]

@mrcoinbase Apologies on the delay, we're having CircleCI issues.

[jamonholmgren]

CircleCI issues are resolved. I think we're ready to merge this. It will be a breaking change.

Actually, our internal team has additional conversations to have here before merging. Sorry about the delay, this is not a straightforward merge.

[mrcoinbase]

@jamonholmgren thanks so much!

[abumalick]

We are really looking forward updating our apps with these changes.

Can you merge and publish it please?

[Titozzz]

I don't believe you should set it to true without adding more logic to the code.

I understand the security issue is annoying, but there is more to be done to mitigate:

Vendors generally have two choices to mitigate for unpatched WebView users:

Enable multiwindow support. If needed, implementation options exist to mimic single-window behavior and minimize breaking changes. Does not require multi-tab UI. Suitable for browsers and frameworks.
or
Keep multiwindow support disabled, and strictly limit WebView rendering to trusted content only. Suitable for non-browser apps, and for frameworks when used in non-browser apps.

The "implementation options exist to mimic single-window behavior and minimize breaking changes." is missing I believe.

Maybe I'm wrong, as I'm not an android expert, far from it :( ! So if you still believe this is not going to break stuff and also fix the issue lmk ❤️

[cmcewen]

Hi! Any help needed here? I would love to fix our yarn audit 🙏

[Titozzz]

Hello everyone again 👋 .

We tried out that pull request on a test project with @jamonholmgren, but setting setSupportMultipleWindows to true without implementing onWindowCreate break current the webview functionality, clicking on a link with target blank won't do anything instead of opening in the current window.

If we want to merge this, we need to come up with an onCreateWindow implementation that opens in the existing window, that way it's not breaking for anyone.

Thanks you in advance for taking time to fix this.

[Titozzz]

See last comment

[jamonholmgren]

To test this, enable supportMulitpleWindows and try any webpage with a link that tries to open a new window. For example, on my website the "blog" link opens in a new tab. Tapping "blog" should open in the existing window.

When we tried to do this using the existing PR, tapping "Blog" simply didn't do anything, which is undesirable.

<WebView
  source={{uri: "https://infinite.red"}}
  style={{width: "100%", height: "100%"}}
/>

[cmcewen]

Thanks for testing y'all. The original post (https://alesandroortiz.com/articles/uxss-android-webview-cve-2020-6506/) links to a Flutter mitigation (https://github.com/flutter/plugins/pull/2991/files) which we can probably adapt. Trying to get a release out, but I can give it a try in a few days if no one else gets to it first

[kelset]

Hey folks, is there any update on when this fix will be merged or an equivalent one will be ready to address the CVE?

[Titozzz]

As long as #1663 (comment) isn't solved, we cannot merge this. Please note that this is a security issue only if users don't have webview up to date AND you load urls you don't know inside your webview, so you can definitely mitigate it by blocking untrustworthy domains if security is your concern. If the goal is just to have the CVE disappear, cause its annoying to have a "security issue" with yarn audit or such, well it's not fixed yet.

[v-almonacid]

@Titozzz sorry if this might be a bit slightly off topic, but do you know if there is a way to create a blacklist for any http/https connections in react native? so that I can make sure that there is no dependency communicating with some server without me being aware of it

[jamonholmgren]

Closing in favor of #1747 -- thank you @mrcoinbase for your work on this (I noted you in the release notes!)

[mrcoinbase]

Thanks! Sorry I didn't have time to devote to fixing those issues. That would be a problem.

Also, @Titozzz agreed on that this closes an inconvenient CVE, but for real security purposes it's better to just not to iframe untrusted domains.