mirror of
https://github.com/bitwarden/android.git
synced 2025-12-11 04:39:19 -06:00
[PM- 22735] Unsafe deserialization parcel data intent (#5419)
Co-authored-by: David Perez <david@livefront.com>
This commit is contained in:
parent
37af6a1773
commit
929233081c
@ -1,8 +1,11 @@
|
||||
package com.x8bit.bitwarden
|
||||
|
||||
import android.app.ComponentCaller
|
||||
import android.content.Intent
|
||||
import android.os.Bundle
|
||||
import androidx.appcompat.app.AppCompatActivity
|
||||
import com.bitwarden.annotation.OmitFromCoverage
|
||||
import com.bitwarden.ui.platform.util.validate
|
||||
|
||||
/**
|
||||
* An activity to be launched and then immediately closed so that the OS Shade can be collapsed
|
||||
@ -11,7 +14,16 @@ import com.bitwarden.annotation.OmitFromCoverage
|
||||
@OmitFromCoverage
|
||||
class AccessibilityActivity : AppCompatActivity() {
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
intent = intent.validate()
|
||||
super.onCreate(savedInstanceState)
|
||||
finish()
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent) {
|
||||
super.onNewIntent(intent.validate())
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent, caller: ComponentCaller) {
|
||||
super.onNewIntent(intent.validate(), caller)
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,10 +1,12 @@
|
||||
package com.x8bit.bitwarden
|
||||
|
||||
import android.app.ComponentCaller
|
||||
import android.content.Intent
|
||||
import android.os.Bundle
|
||||
import androidx.activity.viewModels
|
||||
import androidx.appcompat.app.AppCompatActivity
|
||||
import com.bitwarden.annotation.OmitFromCoverage
|
||||
import com.bitwarden.ui.platform.util.validate
|
||||
import dagger.hilt.android.AndroidEntryPoint
|
||||
|
||||
/**
|
||||
@ -21,6 +23,7 @@ class AuthCallbackActivity : AppCompatActivity() {
|
||||
private val viewModel: AuthCallbackViewModel by viewModels()
|
||||
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
intent = intent.validate()
|
||||
super.onCreate(savedInstanceState)
|
||||
|
||||
viewModel.trySendAction(AuthCallbackAction.IntentReceive(intent = intent))
|
||||
@ -35,4 +38,12 @@ class AuthCallbackActivity : AppCompatActivity() {
|
||||
startActivity(intent)
|
||||
finish()
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent) {
|
||||
super.onNewIntent(intent.validate())
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent, caller: ComponentCaller) {
|
||||
super.onNewIntent(intent.validate(), caller)
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,10 +1,13 @@
|
||||
package com.x8bit.bitwarden
|
||||
|
||||
import android.app.ComponentCaller
|
||||
import android.content.Intent
|
||||
import android.os.Bundle
|
||||
import androidx.activity.viewModels
|
||||
import androidx.appcompat.app.AppCompatActivity
|
||||
import androidx.lifecycle.lifecycleScope
|
||||
import com.bitwarden.annotation.OmitFromCoverage
|
||||
import com.bitwarden.ui.platform.util.validate
|
||||
import com.x8bit.bitwarden.data.autofill.manager.AutofillCompletionManager
|
||||
import dagger.hilt.android.AndroidEntryPoint
|
||||
import kotlinx.coroutines.flow.launchIn
|
||||
@ -26,6 +29,7 @@ class AutofillTotpCopyActivity : AppCompatActivity() {
|
||||
private val autofillTotpCopyViewModel: AutofillTotpCopyViewModel by viewModels()
|
||||
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
intent = intent.validate()
|
||||
super.onCreate(savedInstanceState)
|
||||
|
||||
observeViewModelEvents()
|
||||
@ -37,6 +41,14 @@ class AutofillTotpCopyActivity : AppCompatActivity() {
|
||||
)
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent) {
|
||||
super.onNewIntent(intent.validate())
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent, caller: ComponentCaller) {
|
||||
super.onNewIntent(intent.validate(), caller)
|
||||
}
|
||||
|
||||
private fun observeViewModelEvents() {
|
||||
autofillTotpCopyViewModel
|
||||
.eventFlow
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
package com.x8bit.bitwarden
|
||||
|
||||
import android.app.ComponentCaller
|
||||
import android.content.Intent
|
||||
import android.os.Build
|
||||
import android.os.Bundle
|
||||
@ -23,6 +24,7 @@ import com.bitwarden.annotation.OmitFromCoverage
|
||||
import com.bitwarden.ui.platform.base.util.EventsEffect
|
||||
import com.bitwarden.ui.platform.theme.BitwardenTheme
|
||||
import com.bitwarden.ui.platform.util.setupEdgeToEdge
|
||||
import com.bitwarden.ui.platform.util.validate
|
||||
import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityCompletionManager
|
||||
import com.x8bit.bitwarden.data.autofill.manager.AutofillActivityManager
|
||||
import com.x8bit.bitwarden.data.autofill.manager.AutofillCompletionManager
|
||||
@ -67,6 +69,7 @@ class MainActivity : AppCompatActivity() {
|
||||
lateinit var debugLaunchManager: DebugMenuLaunchManager
|
||||
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
intent = intent.validate()
|
||||
var shouldShowSplashScreen = true
|
||||
installSplashScreen().setKeepOnScreenCondition { shouldShowSplashScreen }
|
||||
super.onCreate(savedInstanceState)
|
||||
@ -114,8 +117,15 @@ class MainActivity : AppCompatActivity() {
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent) {
|
||||
super.onNewIntent(intent)
|
||||
mainViewModel.trySendAction(action = MainAction.ReceiveNewIntent(intent = intent))
|
||||
val newIntent = intent.validate()
|
||||
super.onNewIntent(newIntent)
|
||||
mainViewModel.trySendAction(action = MainAction.ReceiveNewIntent(intent = newIntent))
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent, caller: ComponentCaller) {
|
||||
val newIntent = intent.validate()
|
||||
super.onNewIntent(newIntent, caller)
|
||||
mainViewModel.trySendAction(action = MainAction.ReceiveNewIntent(intent = newIntent))
|
||||
}
|
||||
|
||||
override fun onResume() {
|
||||
|
||||
@ -4,7 +4,7 @@ import android.content.Context
|
||||
import android.content.Intent
|
||||
import com.x8bit.bitwarden.MainActivity
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.PasswordlessRequestData
|
||||
import com.x8bit.bitwarden.data.platform.util.getSafeParcelableExtra
|
||||
import com.bitwarden.ui.platform.util.getSafeParcelableExtra
|
||||
|
||||
private const val NOTIFICATION_DATA: String = "notificationData"
|
||||
|
||||
|
||||
@ -18,7 +18,7 @@ import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo
|
||||
import com.x8bit.bitwarden.data.autofill.model.AutofillSaveItem
|
||||
import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData
|
||||
import com.x8bit.bitwarden.data.autofill.model.AutofillTotpCopyData
|
||||
import com.x8bit.bitwarden.data.platform.util.getSafeParcelableExtra
|
||||
import com.bitwarden.ui.platform.util.getSafeParcelableExtra
|
||||
import kotlin.random.Random
|
||||
|
||||
private const val AUTOFILL_SAVE_ITEM_DATA_KEY = "autofill-save-item-data"
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
package com.bitwarden.authenticator
|
||||
|
||||
import android.app.ComponentCaller
|
||||
import android.content.Intent
|
||||
import android.os.Bundle
|
||||
import android.view.KeyEvent
|
||||
@ -14,13 +15,13 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
import androidx.lifecycle.lifecycleScope
|
||||
import androidx.navigation.NavHostController
|
||||
import androidx.navigation.compose.rememberNavController
|
||||
import com.bitwarden.authenticator.data.platform.util.isSuspicious
|
||||
import com.bitwarden.authenticator.ui.platform.composition.LocalManagerProvider
|
||||
import com.bitwarden.authenticator.ui.platform.feature.debugmenu.manager.DebugMenuLaunchManager
|
||||
import com.bitwarden.authenticator.ui.platform.feature.debugmenu.navigateToDebugMenuScreen
|
||||
import com.bitwarden.authenticator.ui.platform.feature.rootnav.RootNavScreen
|
||||
import com.bitwarden.authenticator.ui.platform.theme.AuthenticatorTheme
|
||||
import com.bitwarden.ui.platform.util.setupEdgeToEdge
|
||||
import com.bitwarden.ui.platform.util.validate
|
||||
import dagger.hilt.android.AndroidEntryPoint
|
||||
import kotlinx.coroutines.flow.launchIn
|
||||
import kotlinx.coroutines.flow.map
|
||||
@ -39,7 +40,7 @@ class MainActivity : AppCompatActivity() {
|
||||
lateinit var debugLaunchManager: DebugMenuLaunchManager
|
||||
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
sanitizeIntent()
|
||||
intent = intent.validate()
|
||||
var shouldShowSplashScreen = true
|
||||
installSplashScreen().setKeepOnScreenCondition { shouldShowSplashScreen }
|
||||
super.onCreate(savedInstanceState)
|
||||
@ -72,20 +73,15 @@ class MainActivity : AppCompatActivity() {
|
||||
}
|
||||
|
||||
override fun onNewIntent(intent: Intent) {
|
||||
super.onNewIntent(intent)
|
||||
sanitizeIntent()
|
||||
mainViewModel.trySendAction(
|
||||
MainAction.ReceiveNewIntent(intent = intent),
|
||||
)
|
||||
val newIntent = intent.validate()
|
||||
super.onNewIntent(newIntent)
|
||||
mainViewModel.trySendAction(MainAction.ReceiveNewIntent(intent = newIntent))
|
||||
}
|
||||
|
||||
private fun sanitizeIntent() {
|
||||
if (intent.isSuspicious) {
|
||||
intent = Intent(
|
||||
/* packageContext = */ this,
|
||||
/* cls = */ MainActivity::class.java,
|
||||
)
|
||||
}
|
||||
override fun onNewIntent(intent: Intent, caller: ComponentCaller) {
|
||||
val newIntent = intent.validate()
|
||||
super.onNewIntent(newIntent, caller)
|
||||
mainViewModel.trySendAction(MainAction.ReceiveNewIntent(intent = newIntent))
|
||||
}
|
||||
|
||||
private fun observeViewModelEvents(navController: NavHostController) {
|
||||
|
||||
@ -1,19 +0,0 @@
|
||||
package com.bitwarden.authenticator.data.platform.util
|
||||
|
||||
import android.content.Intent
|
||||
|
||||
/**
|
||||
* Returns true if this intent contains unexpected or suspicious data.
|
||||
*/
|
||||
val Intent.isSuspicious: Boolean
|
||||
get() {
|
||||
return try {
|
||||
val containsSuspiciousExtras = extras?.isEmpty() == false
|
||||
val containsSuspiciousData = data != null
|
||||
containsSuspiciousData || containsSuspiciousExtras
|
||||
} catch (_: Exception) {
|
||||
// `unparcel()` throws an exception on Android 12 and below if the bundle contains
|
||||
// suspicious data, so we catch the exception and return true.
|
||||
true
|
||||
}
|
||||
}
|
||||
@ -1,64 +0,0 @@
|
||||
package com.bitwarden.authenticator.data.platform.util
|
||||
|
||||
import android.content.Intent
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import org.junit.jupiter.api.Assertions.assertFalse
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
class IntentExtensionsTest {
|
||||
@Test
|
||||
fun `isSuspicious should return true when extras are not empty`() {
|
||||
val intent = mockk<Intent> {
|
||||
every { data } returns mockk()
|
||||
every { extras } returns mockk {
|
||||
every { isEmpty } returns false
|
||||
}
|
||||
}
|
||||
|
||||
assertTrue(intent.isSuspicious)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `isSuspicious should return true when extras are null`() {
|
||||
val intent = mockk<Intent> {
|
||||
every { data } returns mockk()
|
||||
every { extras } returns null
|
||||
}
|
||||
|
||||
assertTrue(intent.isSuspicious)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `isSuspicious should return true when data is not null`() {
|
||||
val intent = mockk<Intent> {
|
||||
every { data } returns mockk()
|
||||
every { extras } returns null
|
||||
}
|
||||
|
||||
assertTrue(intent.isSuspicious)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `isSuspicious should return false when data and extras are null`() {
|
||||
val intent = mockk<Intent> {
|
||||
every { data } returns null
|
||||
every { extras } returns null
|
||||
}
|
||||
|
||||
assertFalse(intent.isSuspicious)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `isSuspicious should return false when data is null and extras are empty`() {
|
||||
val intent = mockk<Intent> {
|
||||
every { data } returns null
|
||||
every { extras } returns mockk {
|
||||
every { isEmpty } returns true
|
||||
}
|
||||
}
|
||||
|
||||
assertFalse(intent.isSuspicious)
|
||||
}
|
||||
}
|
||||
@ -1,8 +1,9 @@
|
||||
@file:OmitFromCoverage
|
||||
|
||||
package com.x8bit.bitwarden.data.platform.util
|
||||
package com.bitwarden.ui.platform.util
|
||||
|
||||
import android.content.Intent
|
||||
import android.os.BadParcelableException
|
||||
import android.os.Bundle
|
||||
import android.os.Parcelable
|
||||
import androidx.core.content.IntentCompat
|
||||
@ -24,3 +25,21 @@ inline fun <reified T> Intent.getSafeParcelableExtra(
|
||||
inline fun <reified T> Bundle.getSafeParcelableExtra(
|
||||
name: String,
|
||||
): T? = BundleCompat.getParcelable(this, name, T::class.java)
|
||||
|
||||
/**
|
||||
* Validate if there's anything suspicious with the intent received and returns a new valid intent.
|
||||
*/
|
||||
fun Intent.validate(): Intent =
|
||||
try {
|
||||
// This will force Android to attempt to fetch each item and verify it is valid
|
||||
this.extras?.let { bundle ->
|
||||
bundle.keySet().forEach { @Suppress("DEPRECATION") bundle.get(it) }
|
||||
}
|
||||
this
|
||||
} catch (_: BadParcelableException) {
|
||||
this.replaceExtras(null as Bundle?)
|
||||
} catch (_: ClassNotFoundException) {
|
||||
this.replaceExtras(null as Bundle?)
|
||||
} catch (@Suppress("TooGenericExceptionCaught") _: RuntimeException) {
|
||||
this.replaceExtras(null as Bundle?)
|
||||
}
|
||||
@ -0,0 +1,68 @@
|
||||
package com.bitwarden.ui.platform.util
|
||||
|
||||
import android.content.Intent
|
||||
import android.os.BadParcelableException
|
||||
import android.os.Bundle
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.mockkStatic
|
||||
import io.mockk.unmockkStatic
|
||||
import io.mockk.verify
|
||||
import org.junit.jupiter.api.AfterEach
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.BeforeEach
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
class IntentExtensionsTest {
|
||||
|
||||
@BeforeEach
|
||||
fun setUp() {
|
||||
mockkStatic(Intent::class)
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
fun tearDown() {
|
||||
unmockkStatic(Intent::class)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validate should not modify the current intent`() {
|
||||
val intent = mockk<Intent>(relaxed = true) {
|
||||
every { getStringExtra("token") } returns "myToken"
|
||||
}
|
||||
|
||||
intent.validate()
|
||||
|
||||
assertEquals("myToken", intent.getStringExtra("token"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validate should remove extras if BadParcelableException is thrown`() {
|
||||
val mockIntent = mockk<Intent>(relaxed = true)
|
||||
|
||||
every { mockIntent.extras } throws BadParcelableException("Bad parcel")
|
||||
|
||||
mockIntent.validate()
|
||||
verify { mockIntent.replaceExtras(null as Bundle?) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validate should remove extras if ClassNotFoundException is thrown`() {
|
||||
val mockIntent = mockk<Intent>(relaxed = true)
|
||||
|
||||
every { mockIntent.extras } throws ClassNotFoundException("Bad parcel")
|
||||
|
||||
mockIntent.validate()
|
||||
verify { mockIntent.replaceExtras(null as Bundle?) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validate should remove extras if RuntimeException is thrown`() {
|
||||
val mockIntent = mockk<Intent>(relaxed = true)
|
||||
|
||||
every { mockIntent.extras } throws RuntimeException("Bad parcel")
|
||||
|
||||
mockIntent.validate()
|
||||
verify { mockIntent.replaceExtras(null as Bundle?) }
|
||||
}
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user