Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce mitigations for CVE-2020-6506 #1747

Merged
merged 6 commits into from Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -13,6 +13,7 @@
import android.net.Uri;
import android.os.Build;
import android.os.Environment;
import android.os.Message;
import android.os.SystemClock;
import android.text.TextUtils;
import android.util.Log;
Expand Down Expand Up @@ -187,6 +188,7 @@ protected WebView createViewInstance(ThemedReactContext reactContext) {
settings.setBuiltInZoomControls(true);
settings.setDisplayZoomControls(false);
settings.setDomStorageEnabled(true);
settings.setSupportMultipleWindows(true);

settings.setAllowFileAccess(false);
settings.setAllowContentAccess(false);
Expand Down Expand Up @@ -252,6 +254,11 @@ public void setJavaScriptEnabled(WebView view, boolean enabled) {
view.getSettings().setJavaScriptEnabled(enabled);
}

@ReactProp(name = "setSupportMultipleWindows")
public void setSupportMultipleWindows(WebView view, boolean enabled){
view.getSettings().setSupportMultipleWindows(enabled);
}

@ReactProp(name = "showsHorizontalScrollIndicator")
public void setShowsHorizontalScrollIndicator(WebView view, boolean enabled) {
view.setHorizontalScrollBarEnabled(enabled);
Expand Down Expand Up @@ -875,7 +882,7 @@ public void onReceivedSslError(final WebView webView, final SslErrorHandler hand
// This is desired behavior. We later use these values to determine whether the request is a top-level navigation or a subresource request.
String topWindowUrl = webView.getUrl();
String failingUrl = error.getUrl();

// Cancel request after obtaining top-level URL.
// If request is cancelled before obtaining top-level URL, undesired behavior may occur.
// Undesired behavior: Return value of WebView.getUrl() may be the current URL instead of the failing URL.
Expand Down Expand Up @@ -1073,6 +1080,17 @@ public RNCWebChromeClient(ReactContext reactContext, WebView webView) {
this.mWebView = webView;
}

@Override
public boolean onCreateWindow(WebView view, boolean isDialog, boolean isUserGesture, Message resultMsg) {

final WebView newWebView = new WebView(view.getContext());
final WebView.WebViewTransport transport = (WebView.WebViewTransport) resultMsg.obj;
transport.setWebView(newWebView);
resultMsg.sendToTarget();

return true;
}

@Override
public boolean onConsoleMessage(ConsoleMessage message) {
if (ReactBuildConfig.DEBUG) {
Expand Down
20 changes: 18 additions & 2 deletions docs/Reference.md
Expand Up @@ -72,6 +72,7 @@ This document lays out the current public properties and methods for the React N
- [`ignoreSilentHardwareSwitch`](Reference.md#ignoreSilentHardwareSwitch)
- [`onFileDownload`](Reference.md#onFileDownload)
- [`autoManageStatusBarEnabled`](Reference.md#autoManageStatusBarEnabled)
- [`setSupportMultipleWindows`](Reference.md#setSupportMultipleWindows)

## Methods Index

Expand Down Expand Up @@ -782,7 +783,7 @@ A Boolean value indicating whether JavaScript can open windows without user inte

### `androidHardwareAccelerationDisabled`[⬆](#props-index)<!-- Link generated with jump2header -->

**Deprecated.** Use the `androidLayerType` prop instead.
**Deprecated.** Use the `androidLayerType` prop instead.

| Type | Required | Platform |
| ---- | -------- | -------- |
Expand All @@ -792,7 +793,7 @@ A Boolean value indicating whether JavaScript can open windows without user inte

### `androidLayerType`[⬆](#props-index)<!-- Link generated with jump2header -->

Specifies the layer type.
Specifies the layer type.

Possible values for `androidLayerType` are:

Expand Down Expand Up @@ -1282,6 +1283,21 @@ Example:
<WebView autoManageStatusBarEnabled={false} />
```

### `setSupportMultipleWindows`

Sets whether the WebView supports multiple windows. See [Android documentation]('https://developer.android.com/reference/android/webkit/WebSettings#setSupportMultipleWindows(boolean)') for more information.
Setting this to false can expose the application to this [vulnerability](https://alesandroortiz.com/articles/uxss-android-webview-cve-2020-6506/) allowing a malicious iframe to escape into the top layer DOM.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until this vulnerability is fixed, would it make sense to prevent people from disabling it?

Copy link
Contributor Author

@kelset kelset Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the vulnerability in Android WebView is already fixed: https://alesandroortiz.com/articles/uxss-android-webview-cve-2020-6506#android-users (this is just a mitigation)

But since we can't really control nor force this kind of update (but I seem to recall google services being quite "aggressive" in self updating) I don't think we should lock it. In a way it, it will be already forced since it's a new setting that will be defaulted to "true", so by simply bumping the webview version you'll have this behaviour. But I think it'd be much better to allow folks who want to revert the behaviour change to be able to do so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with keeping it a flag, at least for now.


| Type | Required | Default | Platform |
| ------- | -------- | ------- | -------- |
| boolean | No | true | Android |

Example:

```javascript
<WebView setSupportMultipleWindows={false} />
```

## Methods

### `extraNativeComponentConfig()`[⬆](#methods-index)<!-- Link generated with jump2header -->
Expand Down
14 changes: 14 additions & 0 deletions example/App.tsx
Expand Up @@ -18,6 +18,7 @@ import Uploads from './examples/Uploads';
import Injection from './examples/Injection';
import LocalPageLoad from './examples/LocalPageLoad';
import Messaging from './examples/Messaging';
import NativeWebpage from './examples/NativeWebpage';

const TESTS = {
Messaging: {
Expand Down Expand Up @@ -84,6 +85,14 @@ const TESTS = {
return <LocalPageLoad />;
},
},
NativeWebpage: {
title: 'NativeWebpage',
testId: 'NativeWebpage',
description: 'Test to open a new webview with a link',
render() {
return <NativeWebpage />;
},
},
};

type Props = {};
Expand Down Expand Up @@ -166,6 +175,11 @@ export default class App extends Component<Props, State> {
title="Messaging"
onPress={() => this._changeTest('Messaging')}
/>
<Button
testID="testType_nativeWebpage"
title="NativeWebpage"
onPress={() => this._changeTest('NativeWebpage')}
/>
</View>

{restarting ? null : (
Expand Down
23 changes: 23 additions & 0 deletions example/examples/NativeWebpage.tsx
@@ -0,0 +1,23 @@
import React, {Component} from 'react';
import {View} from 'react-native';

import WebView from 'react-native-webview';

type Props = {};
type State = {};

export default class NativeWebpage extends Component<Props, State> {
state = {};

render() {
return (
<View style={{height: 400}}>
<WebView
source={{uri: 'https://infinite.red'}}
style={{width: '100%', height: '100%'}}
// setSupportMultipleWindows={false}
/>
</View>
);
}
}
1 change: 1 addition & 0 deletions src/WebView.android.tsx
Expand Up @@ -63,6 +63,7 @@ class WebView extends React.Component<AndroidWebViewProps, State> {
androidHardwareAccelerationDisabled: false,
androidLayerType: 'none',
originWhitelist: defaultOriginWhitelist,
setSupportMultipleWindows: true,
};

static isFileUploadSupported = async () => {
Expand Down
8 changes: 8 additions & 0 deletions src/WebViewTypes.ts
Expand Up @@ -295,6 +295,7 @@ export interface AndroidNativeWebViewProps extends CommonNativeWebViewProps {
onRenderProcessGone?: (event: WebViewRenderProcessGoneEvent) => void;
overScrollMode?: OverScrollModeType;
saveFormDataDisabled?: boolean;
setSupportMultipleWindows?: boolean;
textZoom?: number;
thirdPartyCookiesEnabled?: boolean;
messagingModuleName?: string;
Expand Down Expand Up @@ -799,6 +800,13 @@ export interface AndroidWebViewProps extends WebViewSharedProps {
*/
saveFormDataDisabled?: boolean;

/**
* Boolean value to set whether the WebView supports multiple windows. Used on Android only
* The default value is `true`.
* @platform android
*/
setSupportMultipleWindows?: boolean;

/**
* Used on Android only, controls whether the given list of URL prefixes should
* make {@link com.facebook.react.views.webview.ReactWebViewClient} to launch a
Expand Down