Add additional checks around link preview domains.

We never make requests to non-whitelisted domains, but there were
situations where some links would redirect to non-whitelisted domains,
which would hit a final failsafe that resulted in a crash.

To prevent this, we detect bad redirects earlier and fail more
gracefully.

Fixes #8796
This commit is contained in:
Greyson Parrelli 2019-05-06 12:25:53 -07:00
parent 6c44437c6f
commit 59f362495a
4 changed files with 69 additions and 2 deletions

View File

@ -8,6 +8,7 @@ import com.bumptech.glide.load.model.ModelLoaderFactory;
import com.bumptech.glide.load.model.MultiModelLoaderFactory;
import org.thoughtcrime.securesms.giph.model.ChunkedImageUrl;
import org.thoughtcrime.securesms.net.ContentProxySafetyInterceptor;
import org.thoughtcrime.securesms.net.ContentProxySelector;
import java.io.InputStream;
@ -41,6 +42,7 @@ public class ChunkedImageUrlLoader implements ModelLoader<ChunkedImageUrl, Input
this.client = new OkHttpClient.Builder()
.proxySelector(new ContentProxySelector())
.cache(null)
.addNetworkInterceptor(new ContentProxySafetyInterceptor())
.addNetworkInterceptor(new PaddedHeadersInterceptor())
.build();
}

View File

@ -18,6 +18,7 @@ import org.thoughtcrime.securesms.logging.Log;
import org.thoughtcrime.securesms.mms.GlideApp;
import org.thoughtcrime.securesms.net.CallRequestController;
import org.thoughtcrime.securesms.net.CompositeRequestController;
import org.thoughtcrime.securesms.net.ContentProxySafetyInterceptor;
import org.thoughtcrime.securesms.net.ContentProxySelector;
import org.thoughtcrime.securesms.net.RequestController;
import org.thoughtcrime.securesms.providers.BlobProvider;
@ -49,6 +50,7 @@ public class LinkPreviewRepository {
public LinkPreviewRepository() {
this.client = new OkHttpClient.Builder()
.proxySelector(new ContentProxySelector())
.addNetworkInterceptor(new ContentProxySafetyInterceptor())
.cache(null)
.build();
}

View File

@ -1,6 +1,7 @@
package org.thoughtcrime.securesms.linkpreview;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.text.SpannableString;
import android.text.TextUtils;
import android.text.style.URLSpan;
@ -41,7 +42,9 @@ public final class LinkPreviewUtil {
/**
* @return True if the host is present in the link whitelist.
*/
public static boolean isWhitelistedLinkUrl(@NonNull String linkUrl) {
public static boolean isWhitelistedLinkUrl(@Nullable String linkUrl) {
if (linkUrl == null) return false;
HttpUrl url = HttpUrl.parse(linkUrl);
return url != null &&
!TextUtils.isEmpty(url.scheme()) &&
@ -53,7 +56,9 @@ public final class LinkPreviewUtil {
/**
* @return True if the top-level domain is present in the media whitelist.
*/
public static boolean isWhitelistedMediaUrl(@NonNull String mediaUrl) {
public static boolean isWhitelistedMediaUrl(@Nullable String mediaUrl) {
if (mediaUrl == null) return false;
HttpUrl url = HttpUrl.parse(mediaUrl);
return url != null &&
!TextUtils.isEmpty(url.scheme()) &&

View File

@ -0,0 +1,58 @@
package org.thoughtcrime.securesms.net;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import org.thoughtcrime.securesms.linkpreview.LinkPreviewUtil;
import org.thoughtcrime.securesms.logging.Log;
import java.io.IOException;
import okhttp3.HttpUrl;
import okhttp3.Interceptor;
import okhttp3.Response;
/**
* Interceptor to do extra safety checks on requests through the {@link ContentProxySelector}
* to prevent non-whitelisted requests from getting to it. In particular, this guards against
* requests redirecting to non-whitelisted domains.
*
* Note that because of the way interceptors are ordered, OkHttp will hit the proxy with the
* bad-redirected-domain before we can intercept the request, so we have to "look ahead" by
* detecting a redirected response on the first pass.
*/
public class ContentProxySafetyInterceptor implements Interceptor {
private static final String TAG = Log.tag(ContentProxySafetyInterceptor.class);
@Override
public @NonNull Response intercept(@NonNull Chain chain) throws IOException {
if (isWhitelisted(chain.request().url())) {
Response response = chain.proceed(chain.request());
if (response.isRedirect()) {
if (isWhitelisted(response.header("Location"))) {
return response;
} else {
Log.w(TAG, "Tried to redirect to a non-whitelisted domain!");
chain.call().cancel();
throw new IOException("Tried to redirect to a non-whitelisted domain!");
}
} else {
return response;
}
} else {
Log.w(TAG, "Request was for a non-whitelisted domain!");
chain.call().cancel();
throw new IOException("Request was for a non-whitelisted domain!");
}
}
private static boolean isWhitelisted(@NonNull HttpUrl url) {
return isWhitelisted(url.toString());
}
private static boolean isWhitelisted(@Nullable String url) {
return LinkPreviewUtil.isWhitelistedLinkUrl(url) || LinkPreviewUtil.isWhitelistedMediaUrl(url);
}
}