Skip to content

Commit b3b5530

Browse files
authored
Prevent external intents triggering file sharing (#6734)
Task/Issue URL: https://app.asana.com/1/137249556945/project/608920331025315/task/1211244379062806?focus=true ### Steps to test this PR See linked task Co-authored-by: Craig Russell <[email protected]>
1 parent f679a47 commit b3b5530

File tree

4 files changed

+280
-2
lines changed

4 files changed

+280
-2
lines changed

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM
118118
import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP
119119
import com.duckduckgo.app.browser.refreshpixels.RefreshPixelSender
120120
import com.duckduckgo.app.browser.remotemessage.RemoteMessagingModel
121+
import com.duckduckgo.app.browser.santize.NonHttpAppLinkChecker
121122
import com.duckduckgo.app.browser.senseofprotection.SenseOfProtectionExperiment
122123
import com.duckduckgo.app.browser.session.WebViewSessionStorage
123124
import com.duckduckgo.app.browser.tabs.TabManager
@@ -584,6 +585,8 @@ class BrowserTabViewModelTest {
584585
private val mockOnboardingDesignExperimentManager: OnboardingDesignExperimentManager = mock()
585586
private val mockSerpEasterEggLogoToggles: SerpEasterEggLogosToggles = mock()
586587

588+
private val nonHttpAppLinkChecker: NonHttpAppLinkChecker = mock()
589+
587590
private val EXAMPLE_URL = "http://example.com"
588591
private val SHORT_EXAMPLE_URL = "example.com"
589592

@@ -652,7 +655,7 @@ class BrowserTabViewModelTest {
652655
whenever(mockOnboardingDesignExperimentManager.isBuckEnrolledAndEnabled()).thenReturn(false)
653656
whenever(mockOnboardingDesignExperimentManager.isBbEnrolledAndEnabled()).thenReturn(false)
654657
whenever(mockSerpEasterEggLogoToggles.feature()).thenReturn(mockDisabledToggle)
655-
658+
whenever(nonHttpAppLinkChecker.isPermitted(anyOrNull())).thenReturn(true)
656659
remoteMessagingModel = givenRemoteMessagingModel(mockRemoteMessagingRepository, mockPixel, coroutineRule.testDispatcherProvider)
657660

658661
ctaViewModel = CtaViewModel(
@@ -798,6 +801,7 @@ class BrowserTabViewModelTest {
798801
addressDisplayFormatter = mockAddressDisplayFormatter,
799802
autoCompleteSettings = mockAutoCompleteSettings,
800803
serpEasterEggLogosToggles = mockSerpEasterEggLogoToggles,
804+
nonHttpAppLinkChecker = nonHttpAppLinkChecker,
801805
)
802806

803807
testee.loadData("abc", null, false, false)
@@ -3767,6 +3771,14 @@ class BrowserTabViewModelTest {
37673771
assertCommandIssued<Command.HandleNonHttpAppLink>()
37683772
}
37693773

3774+
@Test
3775+
fun whenHandleNonHttpAppLinkCalledWithUnpermittedIntentThenDoNotHandleNonHttpAppLink() {
3776+
whenever(nonHttpAppLinkChecker.isPermitted(any())).thenReturn(false)
3777+
val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink("market://details?id=com.example", Intent(), EXAMPLE_URL)
3778+
assertTrue(testee.handleNonHttpAppLink(urlType))
3779+
assertCommandNotIssued<Command.HandleNonHttpAppLink>()
3780+
}
3781+
37703782
@Test
37713783
fun whenUserSubmittedQueryIsAppLinkAndShouldShowPromptThenOpenAppLinkInBrowserAndSetPreviousUrlToNull() {
37723784
whenever(mockOmnibarConverter.convertQueryToUrl("foo", null)).thenReturn("foo.com")

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ import com.duckduckgo.app.browser.omnibar.QueryOrigin.FromAutocomplete
193193
import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition
194194
import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP
195195
import com.duckduckgo.app.browser.refreshpixels.RefreshPixelSender
196+
import com.duckduckgo.app.browser.santize.NonHttpAppLinkChecker
196197
import com.duckduckgo.app.browser.senseofprotection.SenseOfProtectionExperiment
197198
import com.duckduckgo.app.browser.session.WebViewSessionStorage
198199
import com.duckduckgo.app.browser.tabs.TabManager
@@ -479,6 +480,7 @@ class BrowserTabViewModel @Inject constructor(
479480
private val addressDisplayFormatter: AddressDisplayFormatter,
480481
private val onboardingDesignExperimentManager: OnboardingDesignExperimentManager,
481482
private val serpEasterEggLogosToggles: SerpEasterEggLogosToggles,
483+
private val nonHttpAppLinkChecker: NonHttpAppLinkChecker,
482484
) : WebViewClientListener,
483485
EditSavedSiteListener,
484486
DeleteBookmarkListener,
@@ -3051,7 +3053,9 @@ class BrowserTabViewModel @Inject constructor(
30513053
}
30523054

30533055
fun nonHttpAppLinkClicked(appLink: NonHttpAppLink) {
3054-
command.value = HandleNonHttpAppLink(appLink, getUrlHeaders(appLink.fallbackUrl))
3056+
if (nonHttpAppLinkChecker.isPermitted(appLink.intent)) {
3057+
command.value = HandleNonHttpAppLink(appLink, getUrlHeaders(appLink.fallbackUrl))
3058+
}
30553059
}
30563060

30573061
fun onPrintSelected() {
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright (c) 2025 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.app.browser.santize
18+
19+
import android.content.Intent
20+
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
21+
import com.duckduckgo.di.scopes.FragmentScope
22+
import com.squareup.anvil.annotations.ContributesBinding
23+
import javax.inject.Inject
24+
import logcat.LogPriority.WARN
25+
import logcat.logcat
26+
27+
interface NonHttpAppLinkChecker {
28+
fun isPermitted(intent: Intent): Boolean
29+
}
30+
31+
@ContributesBinding(FragmentScope::class)
32+
class RealNonHttpAppLinkChecker @Inject constructor(
33+
private val appBuildConfig: AppBuildConfig,
34+
) : NonHttpAppLinkChecker {
35+
36+
override fun isPermitted(intent: Intent): Boolean {
37+
val internalContentFileProvider = CONTENT_PREFIX + appBuildConfig.applicationId + CONTENT_SUFFIX
38+
39+
// check main data URI
40+
if (intent.dataString?.contains(internalContentFileProvider, ignoreCase = true) == true) {
41+
logcat(WARN) { "Refusing to open an internal content URI from external app" }
42+
return false
43+
}
44+
45+
// Check clipData URIs
46+
intent.clipData?.let { clipData ->
47+
for (i in 0 until clipData.itemCount) {
48+
clipData.getItemAt(i).uri?.toString()?.let { uriString ->
49+
if (uriString.contains(internalContentFileProvider, ignoreCase = true)) {
50+
logcat(WARN) { "Refusing to open an internal content URI in clipData from external app" }
51+
return false
52+
}
53+
}
54+
}
55+
}
56+
57+
return true
58+
}
59+
60+
companion object {
61+
private const val CONTENT_PREFIX = "content://"
62+
private const val CONTENT_SUFFIX = ".provider"
63+
}
64+
}
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
/*
2+
* Copyright (c) 2025 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.app.browser.santize
18+
19+
import android.content.ClipData
20+
import android.content.Intent
21+
import android.net.Uri
22+
import androidx.test.ext.junit.runners.AndroidJUnit4
23+
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
24+
import org.junit.Assert.assertFalse
25+
import org.junit.Assert.assertTrue
26+
import org.junit.Before
27+
import org.junit.Test
28+
import org.junit.runner.RunWith
29+
import org.mockito.Mock
30+
import org.mockito.Mockito.mock
31+
import org.mockito.kotlin.whenever
32+
33+
@RunWith(AndroidJUnit4::class)
34+
class RealNonHttpAppLinkCheckerTest {
35+
36+
@Mock
37+
private val appBuildConfig: AppBuildConfig = mock()
38+
39+
private lateinit var testee: RealNonHttpAppLinkChecker
40+
41+
@Before
42+
fun setUp() {
43+
whenever(appBuildConfig.applicationId).thenReturn("com.duckduckgo.mobile.android")
44+
testee = RealNonHttpAppLinkChecker(appBuildConfig)
45+
}
46+
47+
@Test
48+
fun whenIntentDataStringContainsInternalContentFileProviderThenReturnFalse() {
49+
val intent = Intent().apply {
50+
data = Uri.parse("content://com.duckduckgo.mobile.android.provider/path/to/file")
51+
}
52+
53+
val result = testee.isPermitted(intent)
54+
55+
assertFalse(result)
56+
}
57+
58+
@Test
59+
fun whenIntentDataStringContainsInternalContentFileProviderWithSubpathThenReturnFalse() {
60+
val intent = Intent().apply {
61+
data = Uri.parse("content://com.duckduckgo.mobile.android.provider/external_files/documents/test.pdf")
62+
}
63+
64+
val result = testee.isPermitted(intent)
65+
66+
assertFalse(result)
67+
}
68+
69+
@Test
70+
fun whenIntentDataStringContainsExternalContentProviderThenReturnTrue() {
71+
val intent = Intent().apply {
72+
data = Uri.parse("content://com.other.app.provider/path/to/file")
73+
}
74+
75+
val result = testee.isPermitted(intent)
76+
77+
assertTrue(result)
78+
}
79+
80+
@Test
81+
fun whenIntentDataStringIsHttpsUrlThenReturnTrue() {
82+
val intent = Intent().apply {
83+
data = Uri.parse("https://example.com/page")
84+
}
85+
86+
val result = testee.isPermitted(intent)
87+
88+
assertTrue(result)
89+
}
90+
91+
@Test
92+
fun whenIntentDataStringIsCustomSchemeThenReturnTrue() {
93+
val intent = Intent().apply {
94+
data = Uri.parse("myapp://action/data")
95+
}
96+
97+
val result = testee.isPermitted(intent)
98+
99+
assertTrue(result)
100+
}
101+
102+
@Test
103+
fun whenIntentDataIsNullThenReturnTrue() {
104+
val intent = Intent().apply {
105+
data = null
106+
}
107+
108+
val result = testee.isPermitted(intent)
109+
110+
assertTrue(result)
111+
}
112+
113+
@Test
114+
fun whenIntentDataStringIsNullThenReturnTrue() {
115+
val intent = Intent().apply {
116+
data = Uri.parse("")
117+
}
118+
119+
val result = testee.isPermitted(intent)
120+
121+
assertTrue(result)
122+
}
123+
124+
@Test
125+
fun whenIntentDataStringContainsInternalProviderInUpperCaseThenReturnFalse() {
126+
val intent = Intent().apply {
127+
data = Uri.parse("CONTENT://COM.DUCKDUCKGO.MOBILE.ANDROID.PROVIDER/file")
128+
}
129+
130+
assertFalse(testee.isPermitted(intent))
131+
}
132+
133+
@Test
134+
fun whenIntentClipDataContainsInternalContentFileProviderThenReturnFalse() {
135+
val clipData = ClipData.newRawUri("test", Uri.parse("content://com.duckduckgo.mobile.android.provider/file"))
136+
val intent = Intent().apply {
137+
data = Uri.parse("https://example.com/safe")
138+
setClipData(clipData)
139+
}
140+
141+
val result = testee.isPermitted(intent)
142+
143+
assertFalse(result)
144+
}
145+
146+
@Test
147+
fun whenIntentClipDataContainsMultipleUrisWithInternalProviderThenReturnFalse() {
148+
val clipData = ClipData.newRawUri("test", Uri.parse("https://example.com/safe"))
149+
clipData.addItem(ClipData.Item(Uri.parse("content://com.duckduckgo.mobile.android.provider/file")))
150+
val intent = Intent().apply {
151+
data = Uri.parse("https://example.com/safe")
152+
setClipData(clipData)
153+
}
154+
155+
val result = testee.isPermitted(intent)
156+
157+
assertFalse(result)
158+
}
159+
160+
@Test
161+
fun whenIntentClipDataContainsOnlyExternalProvidersThenReturnTrue() {
162+
val clipData = ClipData.newRawUri("test", Uri.parse("content://com.other.app.provider/file"))
163+
clipData.addItem(ClipData.Item(Uri.parse("https://example.com/safe")))
164+
val intent = Intent().apply {
165+
data = Uri.parse("https://example.com/safe")
166+
setClipData(clipData)
167+
}
168+
169+
val result = testee.isPermitted(intent)
170+
171+
assertTrue(result)
172+
}
173+
174+
@Test
175+
fun whenIntentClipDataIsNullThenReturnTrue() {
176+
val intent = Intent().apply {
177+
data = Uri.parse("https://example.com/safe")
178+
clipData = null
179+
}
180+
181+
val result = testee.isPermitted(intent)
182+
183+
assertTrue(result)
184+
}
185+
186+
@Test
187+
fun whenIntentClipDataHasItemWithNullUriThenReturnTrue() {
188+
val clipData = ClipData.newPlainText("test", "some text")
189+
val intent = Intent().apply {
190+
data = Uri.parse("https://example.com/safe")
191+
setClipData(clipData)
192+
}
193+
194+
val result = testee.isPermitted(intent)
195+
196+
assertTrue(result)
197+
}
198+
}

0 commit comments

Comments
 (0)