Skip to content

Commit

Permalink
Make maliciousSiteProtection webView agnostic
Browse files Browse the repository at this point in the history
  • Loading branch information
CrisBarreiro committed Dec 10, 2024
1 parent 7f8a206 commit 13cfb3a
Show file tree
Hide file tree
Showing 10 changed files with 318 additions and 230 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import com.duckduckgo.autoconsent.api.Autoconsent
import com.duckduckgo.autofill.api.BrowserAutofill
import com.duckduckgo.autofill.api.InternalTestUserChecker
import com.duckduckgo.browser.api.JsInjectorPlugin
import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.common.utils.CurrentTimeProvider
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.plugins.PluginPoint
Expand All @@ -76,7 +77,6 @@ import com.duckduckgo.duckplayer.api.DuckPlayer.DuckPlayerState.ENABLED
import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On
import com.duckduckgo.duckplayer.impl.DUCK_PLAYER_OPEN_IN_YOUTUBE_PATH
import com.duckduckgo.history.api.NavigationHistory
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
import com.duckduckgo.privacy.config.api.AmpLinks
import com.duckduckgo.subscriptions.api.Subscriptions
import com.duckduckgo.user.agent.api.ClientBrandHintProvider
Expand Down Expand Up @@ -118,7 +118,7 @@ class BrowserWebViewClient @Inject constructor(
private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector,
private val uriLoadedManager: UriLoadedManager,
private val androidFeaturesHeaderPlugin: AndroidFeaturesHeaderPlugin,
private val phishingAndMalwareDetector: MaliciousSiteProtection,
private val maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
) : WebViewClient() {

var webViewClientListener: WebViewClientListener? = null
Expand Down Expand Up @@ -165,7 +165,15 @@ class BrowserWebViewClient @Inject constructor(
Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url")
webViewClientListener?.onShouldOverride()

if (phishingAndMalwareDetector.shouldOverrideUrlLoading(url, webView.url?.toUri(), isForMainFrame, isRedirect, onSiteBlockedAsync)) {
if (runBlocking {
maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading(
url,
webView.url?.toUri(),
isForMainFrame,
onSiteBlockedAsync,
)
}
) {
// TODO (cbarreiro): Handle site blocked synchronously
return true
}
Expand Down Expand Up @@ -419,7 +427,7 @@ class BrowserWebViewClient @Inject constructor(
// See https://app.asana.com/0/0/1206159443951489/f (WebView limitations)
if (it != ABOUT_BLANK && start == null) {
start = currentTimeProvider.elapsedRealtime()
phishingAndMalwareDetector.onPageLoadStarted()
maliciousSiteProtectionWebViewIntegration.onPageLoadStarted()
}
handleMediaPlayback(webView, it)
autoconsent.injectAutoconsent(webView, url)
Expand Down Expand Up @@ -522,7 +530,13 @@ class BrowserWebViewClient @Inject constructor(
loginDetector.onEvent(WebNavigationEvent.ShouldInterceptRequest(webView, request))
}
Timber.v("Intercepting resource ${request.url} type:${request.method} on page $documentUrl")
requestInterceptor.shouldIntercept(request, webView, documentUrl?.toUri(), phishingAndMalwareDetector, webViewClientListener)
requestInterceptor.shouldIntercept(
request,
webView,
documentUrl?.toUri(),
maliciousSiteProtectionWebViewIntegration,
webViewClientListener,
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ import com.duckduckgo.app.trackerdetection.CloakedCnameDetector
import com.duckduckgo.app.trackerdetection.TrackerDetector
import com.duckduckgo.app.trackerdetection.model.TrackerStatus
import com.duckduckgo.app.trackerdetection.model.TrackingEvent
import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.common.utils.AppUrl
import com.duckduckgo.common.utils.DefaultDispatcherProvider
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.isHttp
import com.duckduckgo.duckplayer.api.DuckPlayer
import com.duckduckgo.httpsupgrade.api.HttpsUpgrader
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
import com.duckduckgo.privacy.config.api.Gpc
import com.duckduckgo.request.filterer.api.RequestFilterer
import com.duckduckgo.user.agent.api.UserAgentProvider
Expand All @@ -49,7 +49,7 @@ interface RequestInterceptor {
request: WebResourceRequest,
webView: WebView,
documentUri: Uri?,
maliciousSiteProtection: MaliciousSiteProtection,
maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
webViewClientListener: WebViewClientListener?,
): WebResourceResponse?

Expand Down Expand Up @@ -94,7 +94,7 @@ class WebViewRequestInterceptor(
request: WebResourceRequest,
webView: WebView,
documentUri: Uri?,
maliciousSiteProtection: MaliciousSiteProtection,
maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
webViewClientListener: WebViewClientListener?,
): WebResourceResponse? {
val url: Uri? = request.url
Expand All @@ -103,7 +103,7 @@ class WebViewRequestInterceptor(
// TODO (cbarreiro): Handle site blocked asynchronously
}

maliciousSiteProtection.shouldIntercept(request, documentUri, onSiteBlockedAsync)?.let {
maliciousSiteProtectionWebViewIntegration.shouldIntercept(request, documentUri, onSiteBlockedAsync)?.let {
// TODO (cbarreiro): Handle site blocked synchronously
return it
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import com.duckduckgo.app.surrogates.ResourceSurrogates
import com.duckduckgo.app.tabs.ui.GridViewColumnCalculator
import com.duckduckgo.app.trackerdetection.CloakedCnameDetector
import com.duckduckgo.app.trackerdetection.TrackerDetector
import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.cookies.api.CookieManagerProvider
import com.duckduckgo.cookies.api.DuckDuckGoCookieManager
Expand All @@ -85,7 +86,6 @@ import com.duckduckgo.downloads.impl.FileDownloadCallback
import com.duckduckgo.duckplayer.api.DuckPlayer
import com.duckduckgo.experiments.api.VariantManager
import com.duckduckgo.httpsupgrade.api.HttpsUpgrader
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
import com.duckduckgo.privacy.config.api.AmpLinks
import com.duckduckgo.privacy.config.api.Gpc
import com.duckduckgo.privacy.config.api.TrackingParameters
Expand Down Expand Up @@ -123,7 +123,7 @@ class BrowserModule {
@AppCoroutineScope appCoroutineScope: CoroutineScope,
dispatcherProvider: DispatcherProvider,
urlExtractor: DOMUrlExtractor,
maliciousSiteProtection: MaliciousSiteProtection,
maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration,
): UrlExtractingWebViewClient {
return UrlExtractingWebViewClient(
webViewHttpAuthStore,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import com.duckduckgo.app.browser.certificates.rootstore.CertificateValidationSt
import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore
import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager
import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore
import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.cookies.api.CookieManagerProvider
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
import kotlinx.coroutines.*
import timber.log.Timber

Expand All @@ -43,7 +43,7 @@ class UrlExtractingWebViewClient(
private val appCoroutineScope: CoroutineScope,
private val dispatcherProvider: DispatcherProvider,
private val urlExtractor: DOMUrlExtractor,
private val maliciousSiteProtection: MaliciousSiteProtection,
private val maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration,
) : WebViewClient() {

var urlExtractionListener: UrlExtractionListener? = null
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Copyright (c) 2024 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.app.browser.webview

import android.net.Uri
import android.webkit.WebResourceRequest
import android.webkit.WebResourceResponse
import androidx.annotation.VisibleForTesting
import androidx.core.net.toUri
import com.duckduckgo.browser.api.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
import com.squareup.anvil.annotations.ContributesBinding
import java.net.URLDecoder
import javax.inject.Inject
import timber.log.Timber

@ContributesBinding(AppScope::class)
class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
private val maliciousSiteProtection: MaliciousSiteProtection,
) : MaliciousSiteBlockerWebViewIntegration {

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
val processedUrls = mutableListOf<String>()

override suspend fun shouldIntercept(
request: WebResourceRequest,
documentUri: Uri?,
onSiteBlockedAsync: () -> Unit,
): WebResourceResponse? {
if (!maliciousSiteProtection.isFeatureEnabled) {
return null
}
val url = request.url.let {
if (it.fragment != null) {
it.buildUpon().fragment(null).build()
} else {
it
}
}

val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase()

if (processedUrls.contains(decodedUrl)) {
processedUrls.remove(decodedUrl)
Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl")
return null
}

if (request.isForMainFrame && decodedUrl.toUri() == documentUri) {
if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) {
return WebResourceResponse(null, null, null)
}
processedUrls.add(decodedUrl)
} else if (isForIframe(request) && documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host) {
if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) {
return WebResourceResponse(null, null, null)
}
processedUrls.add(decodedUrl)
}
return null
}

override suspend fun shouldOverrideUrlLoading(
url: Uri,
webViewUrl: Uri?,
isForMainFrame: Boolean,
onSiteBlockedAsync: () -> Unit,
): Boolean {
if (!maliciousSiteProtection.isFeatureEnabled) {
return false
}
val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase()

if (processedUrls.contains(decodedUrl)) {
processedUrls.remove(decodedUrl)
Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl")
return false
}

if (isForMainFrame && decodedUrl.toUri() == webViewUrl) {
if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), onSiteBlockedAsync)) {
return true
}
processedUrls.add(decodedUrl)
}
return false
}

private fun isForIframe(request: WebResourceRequest) = request.requestHeaders["Sec-Fetch-Dest"] == "iframe" ||
request.url.path?.contains("/embed/") == true ||
request.url.path?.contains("/iframe/") == true ||
request.requestHeaders["Accept"]?.contains("text/html") == true

override fun onPageLoadStarted() {
processedUrls.clear()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package com.duckduckgo.app.browser.webview

import android.webkit.WebResourceRequest
import androidx.core.net.toUri
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
import junit.framework.TestCase.assertTrue
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.mock
import org.mockito.kotlin.any
import org.mockito.kotlin.whenever

@RunWith(AndroidJUnit4::class)
class RealMaliciousSiteBlockerWebViewIntegrationTest {

private lateinit var testee: RealMaliciousSiteBlockerWebViewIntegration
private val maliciousSiteProtection: MaliciousSiteProtection = mock(MaliciousSiteProtection::class.java)
private val maliciousUri = "http://malicious.com".toUri()
private val exampleUri = "http://example.com".toUri()

@Before
fun setup() {
testee = RealMaliciousSiteBlockerWebViewIntegration(maliciousSiteProtection)
whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(true)
}

@Test
fun `shouldOverrideUrlLoading returns false when feature is disabled`() = runTest {
whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(false)

val result = testee.shouldOverrideUrlLoading(exampleUri, null, true) {}
assertFalse(result)
}

@Test
fun `shouldInterceptRequest returns null when feature is disabled`() = runTest {
val request = mock(WebResourceRequest::class.java)
whenever(request.url).thenReturn(exampleUri)
whenever(maliciousSiteProtection.isFeatureEnabled).thenReturn(false)

val result = testee.shouldIntercept(request, null) {}
assertNull(result)
}

@Test
fun `shouldOverrideUrlLoading returns false when url is already processed`() = runTest {
testee.processedUrls.add(exampleUri.toString())

val result = testee.shouldOverrideUrlLoading(exampleUri, exampleUri, true) {}
assertFalse(result)
}

@Test
fun `shouldInterceptRequest returns result when feature is enabled, is malicious, and is mainframe`() = runTest {
val request = mock(WebResourceRequest::class.java)
whenever(request.url).thenReturn(maliciousUri)
whenever(request.isForMainFrame).thenReturn(true)
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true)

val result = testee.shouldIntercept(request, maliciousUri) {}
assertNotNull(result)
}

@Test
fun `shouldInterceptRequest returns result when feature is enabled, is malicious, and is iframe`() = runTest {
val request = mock(WebResourceRequest::class.java)
whenever(request.url).thenReturn(maliciousUri)
whenever(request.isForMainFrame).thenReturn(true)
whenever(request.requestHeaders).thenReturn(mapOf("Sec-Fetch-Dest" to "iframe"))
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true)

val result = testee.shouldIntercept(request, maliciousUri) {}
assertNotNull(result)
}

@Test
fun `shouldInterceptRequest returns null when feature is enabled, is malicious, and is not mainframe nor iframe`() = runTest {
val request = mock(WebResourceRequest::class.java)
whenever(request.url).thenReturn(maliciousUri)
whenever(request.isForMainFrame).thenReturn(false)
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true)

val result = testee.shouldIntercept(request, maliciousUri) {}
assertNull(result)
}

@Test
fun `shouldOverride returns false when feature is enabled, is malicious, and is not mainframe`() = runTest {
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true)

val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {}
assertFalse(result)
}

@Test
fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe`() = runTest {
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true)

val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, true) {}
assertTrue(result)
}

@Test
fun `shouldOverride returns true when feature is enabled, is malicious, and not mainframe nor iframe`() = runTest {
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true)

val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {}
assertFalse(result)
}

@Test
fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe but webView has different host`() = runTest {
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(true)

val result = testee.shouldOverrideUrlLoading(maliciousUri, exampleUri, true) {}
assertFalse(result)
}

@Test
fun `onPageLoadStarted clears processedUrls`() = runTest {
testee.processedUrls.add(exampleUri.toString())
testee.onPageLoadStarted()
assertTrue(testee.processedUrls.isEmpty())
}
}
Loading

0 comments on commit 13cfb3a

Please sign in to comment.