Skip to content

Commit 2796ef7

Browse files
authored
Add Malicious Site Protection algorithm pixels (#5604)
Task/Issue URL: https://app.asana.com/0/488551667048375/1209304069660515 ### Description * Add Malicious Site Protection algorithm pixels ### Steps to test this PR _Feature 1_ - [x] Load https://privacy-test-pages.site/security/badware/phishing-legit-iframe-loader.html - [x] Check m_malicious-site-protection_iframe-loaded with params: {category=phishing} is fired - [x] Check m_malicious-site-protection_error-page-shown is fired _Feature 2_ - [x] Set timeout to 10ms in override suspend fun matches(hashPrefix: String): List<Match> - [x] Load https://privacy-test-pages.site/security/badware/phishing-legit-iframe-loader.html - [x] Check no error page is shown and m_malicious-site-protection_client-timeout is fired _Feature 3_ - [x] Load https://privacy-test-pages.site/security/badware/ - [x] Click on "Phishing iFrame Loader" - [x] Check m_malicious-site-protection_iframe-loaded and m_malicious-site-protection_error-page-shown are fired only once ### UI changes | Before | After | | ------ | ----- | !(Upload before screenshot)|(Upload after screenshot)|
1 parent 3043a93 commit 2796ef7

File tree

10 files changed

+211
-50
lines changed

10 files changed

+211
-50
lines changed

app/src/androidTest/java/com/duckduckgo/app/fakes/FakeMaliciousSiteBlockerWebViewIntegration.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class FakeMaliciousSiteBlockerWebViewIntegration : MaliciousSiteBlockerWebViewIn
4141
return Safe
4242
}
4343

44-
override fun onPageLoadStarted() {
44+
override fun onPageLoadStarted(url: String) {
4545
// no-op
4646
}
4747

app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt

+4
Original file line numberDiff line numberDiff line change
@@ -3210,12 +3210,16 @@ class BrowserTabViewModel @Inject constructor(
32103210
}
32113211

32123212
override fun onReceivedMaliciousSiteWarning(url: Uri, feed: Feed, exempted: Boolean, clientSideHit: Boolean) {
3213+
val previousSite = site
32133214
site = siteFactory.buildSite(url = url.toString(), tabId = tabId)
32143215
site?.maliciousSiteStatus = when (feed) {
32153216
MALWARE -> MaliciousSiteStatus.MALWARE
32163217
PHISHING -> MaliciousSiteStatus.PHISHING
32173218
}
3219+
32183220
if (!exempted) {
3221+
if (currentBrowserViewState().maliciousSiteDetected && previousSite?.url == url.toString()) return
3222+
Timber.d("Received MaliciousSiteWarning for $url, feed: $feed, exempted: false, clientSideHit: $clientSideHit")
32193223
val params = mapOf(CATEGORY_KEY to feed.name.lowercase(), CLIENT_SIDE_HIT_KEY to clientSideHit.toString())
32203224
pixel.fire(AppPixelName.MALICIOUS_SITE_PROTECTION_ERROR_SHOWN, params)
32213225
loadingViewState.postValue(

app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class WebViewRequestInterceptor(
9494

9595
override fun onPageStarted(url: String) {
9696
requestFilterer.registerOnPageCreated(url)
97-
maliciousSiteBlockerWebViewIntegration.onPageLoadStarted()
97+
maliciousSiteBlockerWebViewIntegration.onPageLoadStarted(url)
9898
}
9999

100100
/**

app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt

+59-30
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ import com.duckduckgo.app.browser.webview.ExemptedUrlsHolder.ExemptedUrl
2424
import com.duckduckgo.app.browser.webview.RealMaliciousSiteBlockerWebViewIntegration.IsMaliciousViewData
2525
import com.duckduckgo.app.di.AppCoroutineScope
2626
import com.duckduckgo.app.di.IsMainProcess
27+
import com.duckduckgo.app.pixels.AppPixelName
2728
import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature
2829
import com.duckduckgo.app.settings.db.SettingsDataStore
30+
import com.duckduckgo.app.statistics.pixels.Pixel
2931
import com.duckduckgo.common.utils.DispatcherProvider
3032
import com.duckduckgo.di.scopes.AppScope
3133
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
@@ -61,7 +63,7 @@ interface MaliciousSiteBlockerWebViewIntegration {
6163
confirmationCallback: (maliciousStatus: MaliciousStatus) -> Unit,
6264
): IsMaliciousViewData
6365

64-
fun onPageLoadStarted()
66+
fun onPageLoadStarted(url: String)
6567

6668
fun onSiteExempted(
6769
url: Uri,
@@ -97,10 +99,11 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
9799
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
98100
private val exemptedUrlsHolder: ExemptedUrlsHolder,
99101
@IsMainProcess private val isMainProcess: Boolean,
102+
private val pixel: Pixel,
100103
) : MaliciousSiteBlockerWebViewIntegration, PrivacyConfigCallbackPlugin {
101104

102105
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
103-
val processedUrls = mutableListOf<String>()
106+
val processedUrls = mutableMapOf<String, MaliciousStatus>()
104107

105108
private var isFeatureEnabled = false
106109
private val isSettingEnabled: Boolean
@@ -137,6 +140,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
137140
if (!isEnabled()) {
138141
return IsMaliciousViewData.Safe
139142
}
143+
140144
val url = request.url.let {
141145
if (it.fragment != null) {
142146
it.buildUpon().fragment(null).build()
@@ -147,35 +151,49 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
147151

148152
val decodedUrl = decodeUrl(url)
149153

150-
if (processedUrls.contains(decodedUrl)) {
151-
processedUrls.remove(decodedUrl)
152-
Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl")
153-
return IsMaliciousViewData.Safe
154-
}
155-
156154
val exemptedUrl = exemptedUrlsHolder.exemptedMaliciousUrls.firstOrNull { it.url.toString() == decodedUrl }
157155

158156
if (exemptedUrl != null) {
159-
Timber.tag("MaliciousSiteDetector").d("Previously exempted, skipping $decodedUrl as ${exemptedUrl.feed}")
157+
Timber.d("Previously exempted, skipping $decodedUrl as ${exemptedUrl.feed}")
160158
return IsMaliciousViewData.MaliciousSite(url, exemptedUrl.feed, true)
161159
}
162160

161+
processedUrls[decodedUrl]?.let {
162+
processedUrls.remove(decodedUrl)
163+
Timber.d("Already intercepted, skipping $decodedUrl, status: $it")
164+
return when (it) {
165+
is Safe -> IsMaliciousViewData.Safe
166+
is Malicious -> IsMaliciousViewData.MaliciousSite(url, it.feed, false)
167+
}
168+
}
169+
163170
val belongsToCurrentPage = documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host
164-
if (request.isForMainFrame || (isForIframe(request) && belongsToCurrentPage)) {
165-
when (val result = checkMaliciousUrl(decodedUrl, confirmationCallback)) {
171+
val isForIframe = isForIframe(request) && belongsToCurrentPage
172+
if (request.isForMainFrame || isForIframe) {
173+
val result = checkMaliciousUrl(decodedUrl) {
174+
if (isForIframe && it is Malicious) {
175+
firePixelForMaliciousIframe(it.feed)
176+
}
177+
confirmationCallback(it)
178+
}
179+
when (result) {
166180
is ConfirmedResult -> {
181+
processedUrls[decodedUrl] = result.status
167182
when (val status = result.status) {
168183
is Malicious -> {
184+
if (isForIframe) {
185+
firePixelForMaliciousIframe(status.feed)
186+
}
169187
return IsMaliciousViewData.MaliciousSite(url, status.feed, false)
170188
}
189+
171190
is Safe -> {
172-
processedUrls.add(decodedUrl)
173191
return IsMaliciousViewData.Safe
174192
}
175193
}
176194
}
195+
177196
is WaitForConfirmation -> {
178-
processedUrls.add(decodedUrl)
179197
return IsMaliciousViewData.WaitForConfirmation
180198
}
181199
}
@@ -194,19 +212,22 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
194212
}
195213
val decodedUrl = decodeUrl(url)
196214

197-
if (processedUrls.contains(decodedUrl)) {
198-
processedUrls.remove(decodedUrl)
199-
Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl")
200-
return@runBlocking IsMaliciousViewData.Safe
201-
}
202-
203215
val exemptedUrl = exemptedUrlsHolder.exemptedMaliciousUrls.firstOrNull { it.url.toString() == decodedUrl }
204216

205217
if (exemptedUrl != null) {
206-
Timber.tag("MaliciousSiteDetector").d("Previously exempted, skipping $decodedUrl")
218+
Timber.d("Previously exempted, skipping $decodedUrl")
207219
return@runBlocking IsMaliciousViewData.MaliciousSite(url, exemptedUrl.feed, true)
208220
}
209221

222+
processedUrls[decodedUrl]?.let {
223+
processedUrls.remove(decodedUrl)
224+
Timber.d("Already intercepted, skipping $decodedUrl, status: $it")
225+
return@runBlocking when (it) {
226+
is Safe -> IsMaliciousViewData.Safe
227+
is Malicious -> IsMaliciousViewData.MaliciousSite(url, it.feed, false)
228+
}
229+
}
230+
210231
// iframes always go through the shouldIntercept method, so we only need to check the main frame here
211232
if (isForMainFrame) {
212233
when (val result = checkMaliciousUrl(decodedUrl, confirmationCallback)) {
@@ -216,13 +237,12 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
216237
return@runBlocking IsMaliciousViewData.MaliciousSite(url, status.feed, false)
217238
}
218239
is Safe -> {
219-
processedUrls.add(decodedUrl)
240+
processedUrls[decodedUrl] = Safe
220241
return@runBlocking IsMaliciousViewData.Safe
221242
}
222243
}
223244
}
224245
is WaitForConfirmation -> {
225-
processedUrls.add(decodedUrl)
226246
return@runBlocking IsMaliciousViewData.WaitForConfirmation
227247
}
228248
}
@@ -231,6 +251,10 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
231251
}
232252
}
233253

254+
private fun firePixelForMaliciousIframe(feed: Feed) {
255+
pixel.fire(AppPixelName.MALICIOUS_SITE_DETECTED_IN_IFRAME, mapOf("category" to feed.name.lowercase()))
256+
}
257+
234258
private suspend fun checkMaliciousUrl(
235259
url: String,
236260
confirmationCallback: (maliciousStatus: MaliciousStatus) -> Unit,
@@ -243,7 +267,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
243267
} else {
244268
Safe
245269
}
246-
processedUrls.clear()
270+
processedUrls[url] = it
247271
confirmationCallback(isMalicious)
248272
}
249273
}
@@ -257,6 +281,17 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
257281
return isFeatureEnabled && isSettingEnabled
258282
}
259283

284+
override fun onPageLoadStarted(url: String) {
285+
val convertedUrl = URLDecoder.decode(url, "UTF-8").lowercase()
286+
/* onPageLoadStarted is often called after shouldOverride/shouldIntercept, therefore, if the URL
287+
* is already stored, we don't clear the processedUrls map to avoid re-checking the URL for the same
288+
* page load.
289+
*/
290+
if (!processedUrls.contains(convertedUrl)) {
291+
processedUrls.clear()
292+
}
293+
}
294+
260295
private fun decodeUrl(url: Uri): String {
261296
return try {
262297
URLDecoder.decode(url.toString(), "UTF-8").lowercase()
@@ -266,18 +301,12 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
266301
}
267302
}
268303

269-
override fun onPageLoadStarted() {
270-
processedUrls.clear()
271-
}
272-
273304
override fun onSiteExempted(
274305
url: Uri,
275306
feed: Feed,
276307
) {
277308
val convertedUrl = decodeUrl(url)
278309
exemptedUrlsHolder.addExemptedMaliciousUrl(ExemptedUrl(convertedUrl.toUri(), feed))
279-
Timber.tag("MaliciousSiteDetector").d(
280-
"Added $url to exemptedUrls, contents: ${exemptedUrlsHolder.exemptedMaliciousUrls}",
281-
)
310+
Timber.d("Added $url to exemptedUrls")
282311
}
283312
}

app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt

+2
Original file line numberDiff line numberDiff line change
@@ -392,4 +392,6 @@ enum class AppPixelName(override val pixelName: String) : Pixel.PixelName {
392392
SET_AS_DEFAULT_PROMPT_CLICK("m_set-as-default_prompt_click"),
393393
SET_AS_DEFAULT_PROMPT_DISMISSED("m_set-as-default_prompt_dismissed"),
394394
SET_AS_DEFAULT_IN_MENU_CLICK("m_set-as-default_in-menu_click"),
395+
396+
MALICIOUS_SITE_DETECTED_IN_IFRAME("m_malicious-site-protection_iframe-loaded"),
395397
}

0 commit comments

Comments
 (0)