Skip to content

Commit

Permalink
This ensures no illegal cookies are send to okhttp
Browse files Browse the repository at this point in the history
Summary:
When a website in a ReactNative WebView sets a cookie with an illegal
character, this cookie will automatically be added to any request to the
same domain.

This happens through:
BridgeInterceptor.java (l.84)
ReactCookieJarContainer.java (l.44)
JavaNetCookieJar.java (l.59)
ForwardingCookieHandler.java (l.57)
ForwardingCookieHandler.java (l.168)
CookieManager.java (l.39)

The BridgeInterceptor.java then tries to set a Cookie header, which
validates both keys and values, and then crashes.
okhttp3.6.0 Headers.java (l.320)

This fix will strip illegal characters from any cookie that is being
passed to the okhttp request.

To demonstrate how to crash the app, you can find an example app here:
https://github.com/erikpoort/react-native-test-illegal-cookie

Or you can load the following url into a webview: https://invalidcookietest.us.dev.monkapps.com/
Press the 'Set cookie' button.
Then try to fetch the same url.

[ANDROID] [BREAKING] [ReactCookieJarContainer.java] - I'm filtering cookies containing illegal characters from any request.
Closes #18203

Differential Revision: D8164302

Pulled By: hramos

fbshipit-source-id: 6e58461df594eb2c7aad4c7ad70b76d12ac09b84
  • Loading branch information
Erik Poort authored and facebook-github-bot committed May 25, 2018
1 parent 41975f7 commit 04028bf
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package com.facebook.react.modules.network;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import javax.annotation.Nullable;

import okhttp3.Cookie;
import okhttp3.CookieJar;
import okhttp3.Headers;
import okhttp3.HttpUrl;

/**
Expand Down Expand Up @@ -37,7 +39,17 @@ public void saveFromResponse(HttpUrl url, List<Cookie> cookies) {
@Override
public List<Cookie> loadForRequest(HttpUrl url) {
if (cookieJar != null) {
return cookieJar.loadForRequest(url);
List<Cookie> cookies = cookieJar.loadForRequest(url);
ArrayList<Cookie> validatedCookies = new ArrayList<>();
for (Cookie cookie : cookies) {
try {
Headers.Builder cookieChecker = new Headers.Builder();
cookieChecker.add(cookie.name(), cookie.value());
validatedCookies.add(cookie);
} catch (IllegalArgumentException ignored) {
}
}
return validatedCookies;
}
return Collections.emptyList();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.modules.network;

import com.facebook.react.modules.network.ReactCookieJarContainer;
import okhttp3.Cookie;
import okhttp3.CookieJar;
import okhttp3.HttpUrl;
import java.util.List;
import java.util.ArrayList;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.robolectric.RobolectricTestRunner;

import static org.fest.assertions.api.Assertions.assertThat;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
* Tests for {@link NetworkingModule}.
*/
@PrepareForTest({
ReactCookieJarContainer.class
})
@RunWith(RobolectricTestRunner.class)
@PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "android.*"})

public class ReactCookieJarContainerTest {

@Test
public void testMissingJar() throws Exception {
ReactCookieJarContainer jarContainer = mock(ReactCookieJarContainer.class);
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(0);
}

@Test
public void testEmptyCookies() throws Exception {
ReactCookieJarContainer jarContainer = mock(ReactCookieJarContainer.class);
List<Cookie> cookies = new ArrayList<>();
when(jarContainer.loadForRequest(any(HttpUrl.class))).thenReturn(cookies);
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(0);
}

@Test
public void testValidCookies() throws Exception {
ReactCookieJarContainer jarContainer = new ReactCookieJarContainer();
CookieJar cookieJar = mock(CookieJar.class);
jarContainer.setCookieJar(cookieJar);
List<Cookie> cookies = new ArrayList<>();
cookies.add(new Cookie.Builder()
.name("valid")
.value("valid value")
.domain("domain")
.build()
);
when(cookieJar.loadForRequest(any(HttpUrl.class))).thenReturn(cookies);
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(1);
}

@Test
public void testInvalidCookies() throws Exception {
ReactCookieJarContainer jarContainer = new ReactCookieJarContainer();
CookieJar cookieJar = mock(CookieJar.class);
jarContainer.setCookieJar(cookieJar);
List<Cookie> cookies = new ArrayList<>();
cookies.add(new Cookie.Builder()
.name("valid")
.value("înválíd välūė")
.domain("domain")
.build()
);
when(cookieJar.loadForRequest(any(HttpUrl.class))).thenReturn(cookies);
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(0);
}
}

0 comments on commit 04028bf

Please sign in to comment.