For #4686: Fix potential security issue (#4764)

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -160,7 +160,7 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs
                 findInPageLauncher = { findInPageIntegration.withFeature { it.launch() } },
                 nestedScrollQuickActionView = nestedScrollQuickAction,
                 engineView = engineView,
-                currentSession = session,
+                customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) },
                 viewModel = viewModel,
                 getSupportUrl = {
                     SupportUtils.getSumoURLForTopic(
@@ -177,12 +177,14 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs
             )
 
             browserInteractor =
-                createBrowserToolbarViewInteractor(browserToolbarController, session)
+                createBrowserToolbarViewInteractor(
+                    browserToolbarController,
+                    customTabSessionId?.let { sessionManager.findSessionById(it) })
 
             browserToolbarView = BrowserToolbarView(
                 container = view.browserLayout,
                 interactor = browserInteractor,
-                currentSession = session
+                customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) }
             )
 
             toolbarIntegration.set(
@@ -476,7 +478,7 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs
 
     protected abstract fun createBrowserToolbarViewInteractor(
         browserToolbarController: BrowserToolbarController,
-        session: Session
+        session: Session?
     ): BrowserInteractor
 
     /**

app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt

@@ -192,7 +192,7 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler {
 
     override fun createBrowserToolbarViewInteractor(
         browserToolbarController: BrowserToolbarController,
-        session: Session
+        session: Session?
     ) = BrowserInteractor(
         context = context!!,
         store = browserStore,
@@ -208,7 +208,7 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler {
             }
         ),
         readerModeController = DefaultReaderModeController(readerViewFeature),
-        currentSession = session
+        customTabSession = customTabSessionId?.let { requireComponents.core.sessionManager.findSessionById(it) }
     )
 
     override fun getEngineMargins(): Pair<Int, Int> {

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt

@@ -8,6 +8,7 @@ import android.content.Context
 import mozilla.components.browser.session.Session
 import org.mozilla.fenix.browser.readermode.ReaderModeController
 import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.quickactionsheet.QuickActionSheetController
 import org.mozilla.fenix.quickactionsheet.QuickActionSheetViewInteractor
@@ -18,7 +19,7 @@ class BrowserInteractor(
     private val browserToolbarController: BrowserToolbarController,
     private val quickActionSheetController: QuickActionSheetController,
     private val readerModeController: ReaderModeController,
-    private val currentSession: Session
+    private val customTabSession: Session?
 ) : BrowserToolbarViewInteractor, QuickActionSheetViewInteractor {
 
     override fun onBrowserToolbarClicked() {
@@ -50,7 +51,8 @@ class BrowserInteractor(
     }
 
     override fun onQuickActionSheetReadPressed() {
-        val enabled = currentSession.readerMode
+        val enabled =
+            customTabSession?.readerMode ?: context.components.core.sessionManager.selectedSession?.readerMode ?: false
 
         if (enabled) {
             context.metrics.track(Event.QuickActionSheetClosed)

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt

@@ -43,7 +43,7 @@ class DefaultBrowserToolbarController(
     private val findInPageLauncher: () -> Unit,
     private val nestedScrollQuickActionView: NestedScrollView,
     private val engineView: EngineView,
-    private val currentSession: Session,
+    private val customTabSession: Session?,
     private val viewModel: CreateCollectionViewModel,
     private val getSupportUrl: () -> String,
     private val openInFenixIntent: Intent,
@@ -58,7 +58,7 @@ class DefaultBrowserToolbarController(
         navController.nav(
             R.id.browserFragment,
             BrowserFragmentDirections.actionBrowserFragmentToSearchFragment(
-                currentSession.id
+                customTabSession?.id
             )
         )
     }
@@ -69,6 +69,7 @@ class DefaultBrowserToolbarController(
     override fun handleToolbarItemInteraction(item: ToolbarMenu.Item) {
         val sessionUseCases = context.components.useCases.sessionUseCases
         trackToolbarItemInteraction(item)
+        val currentSession = customTabSession ?: context.components.core.sessionManager.selectedSession
 
         Do exhaustive when (item) {
             ToolbarMenu.Item.Back -> sessionUseCases.goBack.invoke(currentSession)
@@ -88,7 +89,8 @@ class DefaultBrowserToolbarController(
                 currentSession
             )
             ToolbarMenu.Item.Share -> {
-                currentSession.url.apply {
+                val currentUrl = currentSession?.url
+                currentUrl?.apply {
                     val directions = BrowserFragmentDirections.actionBrowserFragmentToShareFragment(this)
                     navController.nav(R.id.browserFragment, directions)
                 }
@@ -109,7 +111,8 @@ class DefaultBrowserToolbarController(
                 context.components.analytics.metrics.track(Event.FindInPageOpened)
             }
             ToolbarMenu.Item.ReportIssue -> {
-                currentSession.url.apply {
+                val currentUrl = currentSession?.url
+                currentUrl?.apply {
                     val reportUrl = String.format(BrowserFragment.REPORT_SITE_ISSUE_URL, this)
                     context.components.useCases.tabsUseCases.addTab.invoke(reportUrl)
                 }
@@ -121,25 +124,25 @@ class DefaultBrowserToolbarController(
                 context.components.analytics.metrics
                     .track(Event.CollectionSaveButtonPressed(TELEMETRY_BROWSER_IDENTIFIER))
 
-                    viewModel.tabs = listOf(currentSessionAsTab)
+                viewModel.tabs = listOf(currentSessionAsTab)
-                    val selectedSet = mutableSetOf(currentSessionAsTab)
+                val selectedSet = mutableSetOf(currentSessionAsTab)
-                    viewModel.selectedTabs = selectedSet
+                viewModel.selectedTabs = selectedSet
-                    viewModel.tabCollections =
+                viewModel.tabCollections =
-                        context.components.core.tabCollectionStorage.cachedTabCollections.reversed()
+                    context.components.core.tabCollectionStorage.cachedTabCollections.reversed()
-                    viewModel.saveCollectionStep = viewModel.tabCollections.getStepForCollectionsSize()
+                viewModel.saveCollectionStep = viewModel.tabCollections.getStepForCollectionsSize()
-                    viewModel.snackbarAnchorView = nestedScrollQuickActionView
+                viewModel.snackbarAnchorView = nestedScrollQuickActionView
-                    viewModel.previousFragmentId = R.id.browserFragment
+                viewModel.previousFragmentId = R.id.browserFragment
 
-                    val directions = BrowserFragmentDirections.actionBrowserFragmentToCreateCollectionFragment()
+                val directions = BrowserFragmentDirections.actionBrowserFragmentToCreateCollectionFragment()
-                    navController.nav(R.id.browserFragment, directions)
+                navController.nav(R.id.browserFragment, directions)
             }
             ToolbarMenu.Item.OpenInFenix -> {
                 // Release the session from this view so that it can immediately be rendered by a different view
                 engineView.release()
 
                 // Strip the CustomTabConfig to turn this Session into a regular tab and then select it
-                currentSession.customTabConfig = null
+                customTabSession!!.customTabConfig = null
-                context.components.core.sessionManager.select(currentSession)
+                context.components.core.sessionManager.select(customTabSession)
 
                 // Switch to the actual browser which should now display our new selected session
                 context.startActivity(openInFenixIntent)

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt

@@ -23,7 +23,7 @@ interface BrowserToolbarViewInteractor {
 class BrowserToolbarView(
     private val container: ViewGroup,
     private val interactor: BrowserToolbarViewInteractor,
-    private val currentSession: Session
+    private val customTabSession: Session?
 ) : LayoutContainer {
 
     override val containerView: View?
@@ -41,7 +41,7 @@ class BrowserToolbarView(
     init {
         with(container.context) {
             val sessionManager = components.core.sessionManager
-            val isCustomTabSession = currentSession.isCustomTabSession()
+            val isCustomTabSession = customTabSession != null
 
             view.apply {
                 elevation = TOOLBAR_ELEVATION.dpToFloat(resources.displayMetrics)
@@ -80,7 +80,7 @@ class BrowserToolbarView(
                 CustomTabToolbarMenu(
                     this,
                     sessionManager,
-                    currentSession.id,
+                    customTabSession?.id,
                     onItemTapped = {
                         interactor.onBrowserToolbarMenuItemTapped(it)
                     }
@@ -89,7 +89,9 @@ class BrowserToolbarView(
                 DefaultToolbarMenu(
                     this,
                     hasAccountProblem = components.backgroundServices.accountManager.accountNeedsReauth(),
-                    requestDesktopStateProvider = { currentSession.desktopMode },
+                    requestDesktopStateProvider = {
+                        sessionManager.selectedSession?.private ?: false
+                    },
                     onItemTapped = { interactor.onBrowserToolbarMenuItemTapped(it) }
                 )
             }
@@ -102,8 +104,8 @@ class BrowserToolbarView(
                 ShippedDomainsProvider().also { it.initialize(this) },
                 components.core.historyStorage,
                 components.core.sessionManager,
-                currentSession.id,
+                customTabSession?.id,
-                currentSession.private
+                customTabSession?.private ?: sessionManager.selectedSession?.private ?: false
             )
         }
     }

app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt

@@ -69,7 +69,7 @@ class DefaultBrowserToolbarControllerTest {
             findInPageLauncher = findInPageLauncher,
             nestedScrollQuickActionView = nestedScrollQuickActionView,
             engineView = engineView,
-            currentSession = currentSession,
+            customTabSession = null,
             viewModel = viewModel,
             getSupportUrl = getSupportUrl,
             openInFenixIntent = openInFenixIntent,
@@ -80,6 +80,7 @@ class DefaultBrowserToolbarControllerTest {
         every { context.components.analytics } returns analytics
         every { analytics.metrics } returns metrics
         every { context.components.useCases.sessionUseCases } returns sessionUseCases
+        every { context.components.core.sessionManager.selectedSession } returns currentSession
     }
 
     @Test
@@ -89,10 +90,12 @@ class DefaultBrowserToolbarControllerTest {
         controller.handleToolbarClick()
 
         verify { metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER)) }
-        verify { navController.nav(
+        verify {
-            R.id.browserFragment,
+            navController.nav(
-            BrowserFragmentDirections.actionBrowserFragmentToSearchFragment(currentSession.id)
+                R.id.browserFragment,
-        ) }
+                BrowserFragmentDirections.actionBrowserFragmentToSearchFragment("1")
+            )
+        }
     }
 
     @Test
@@ -144,11 +147,14 @@ class DefaultBrowserToolbarControllerTest {
         controller.handleToolbarItemInteraction(item)
 
         verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SETTINGS)) }
-        verify { navController.nav(
+        verify {
-            R.id.settingsFragment,
+            navController.nav(
-            BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment()
+                R.id.settingsFragment,
-        ) }
+                BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment()
+            )
+        }
     }
+
     @Test
     fun handleToolbarLibraryPress() {
         val item = ToolbarMenu.Item.Library
@@ -156,10 +162,12 @@ class DefaultBrowserToolbarControllerTest {
         controller.handleToolbarItemInteraction(item)
 
         verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.LIBRARY)) }
-        verify { navController.nav(
+        verify {
-            R.id.libraryFragment,
+            navController.nav(
-            BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment()
+                R.id.libraryFragment,
-        ) }
+                BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment()
+            )
+        }
     }
 
     @Test
@@ -248,6 +256,7 @@ class DefaultBrowserToolbarControllerTest {
 
         val item = ToolbarMenu.Item.ReportIssue
 
+        every { currentSession.id } returns "1"
         every { currentSession.url } returns "https://mozilla.org"
         every { context.components.useCases.tabsUseCases } returns tabsUseCases
         every { tabsUseCases.addTab } returns addTabUseCase
@@ -322,6 +331,21 @@ class DefaultBrowserToolbarControllerTest {
 
     @Test
     fun handleToolbarOpenInFenixPress() {
+        controller = DefaultBrowserToolbarController(
+            context = context,
+            navController = navController,
+            browsingModeManager = browsingModeManager,
+            findInPageLauncher = findInPageLauncher,
+            nestedScrollQuickActionView = nestedScrollQuickActionView,
+            engineView = engineView,
+            customTabSession = currentSession,
+            viewModel = viewModel,
+            getSupportUrl = getSupportUrl,
+            openInFenixIntent = openInFenixIntent,
+            currentSessionAsTab = currentSessionAsTab,
+            bottomSheetBehavior = bottomSheetBehavior
+        )
+
         val sessionManager: SessionManager = mockk(relaxed = true)
         val item = ToolbarMenu.Item.OpenInFenix