From 9a66ff38d1015142d8295bc360ee7c04cfa507c5 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 6 Feb 2026 15:57:16 +0000 Subject: [PATCH] Fix security vulnerability: Validate external intent URLs and require confirmation - Remove automatic library addition for external `ACTION_VIEW` intents. - Implement `UrlSecurity` to block private IPs and non-HTTP schemes (SSRF mitigation). - Add `ExternalUrlConfirmationDialog` to prompt user consent before loading external links. - Hoist dialog state to `MainActivity` to ensure visibility regardless of current screen. - Update `ReaderViewModel` to manage external URL request state. Co-authored-by: Aatricks <113598245+Aatricks@users.noreply.github.com> --- .../io/aatricks/novelscraper/MainActivity.kt | 23 ++++++++-- .../ExternalUrlConfirmationDialog.kt | 43 +++++++++++++++++++ .../novelscraper/ui/screens/ReaderScreen.kt | 1 + .../ui/viewmodel/ReaderViewModel.kt | 26 ++++++++++- .../aatricks/novelscraper/util/UrlSecurity.kt | 40 +++++++++++++++++ .../novelscraper/util/UrlSecurityTest.kt | 37 ++++++++++++++++ 6 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 app/src/main/java/io/aatricks/novelscraper/ui/components/ExternalUrlConfirmationDialog.kt create mode 100644 app/src/main/java/io/aatricks/novelscraper/util/UrlSecurity.kt create mode 100644 app/src/test/java/io/aatricks/novelscraper/util/UrlSecurityTest.kt diff --git a/app/src/main/java/io/aatricks/novelscraper/MainActivity.kt b/app/src/main/java/io/aatricks/novelscraper/MainActivity.kt index 2dcaaee..2cab2dd 100644 --- a/app/src/main/java/io/aatricks/novelscraper/MainActivity.kt +++ b/app/src/main/java/io/aatricks/novelscraper/MainActivity.kt @@ -79,6 +79,25 @@ class MainActivity : ComponentActivity() { dynamicColor = true ) { val navController = rememberNavController() + val readerUiState by readerViewModel.uiState.collectAsState() + + if (readerUiState.showExternalUrlConfirmation && readerUiState.pendingExternalUrl != null) { + io.aatricks.novelscraper.ui.components.ExternalUrlConfirmationDialog( + url = readerUiState.pendingExternalUrl!!, + onConfirm = { + readerViewModel.confirmExternalUrl() + navController.navigate(ReaderRoute) { + popUpTo(navController.graph.startDestinationId) { + saveState = true + } + launchSingleTop = true + restoreState = true + } + }, + onCancel = { readerViewModel.cancelExternalUrl() } + ) + } + NavHost(navController = navController, startDestination = ReaderRoute) { composable { ReaderScreen( @@ -144,9 +163,7 @@ class MainActivity : ComponentActivity() { } private fun handleWebUrl(url: String): Unit { - val title = io.aatricks.novelscraper.util.TextUtils.extractTitleFromUrl(url) - libraryViewModel.addItem(title = title, url = url, contentType = ContentType.WEB) - readerViewModel.loadContent(url) + readerViewModel.requestOpenUrl(url) } private fun handleFilePicked(uri: Uri): Unit { diff --git a/app/src/main/java/io/aatricks/novelscraper/ui/components/ExternalUrlConfirmationDialog.kt b/app/src/main/java/io/aatricks/novelscraper/ui/components/ExternalUrlConfirmationDialog.kt new file mode 100644 index 0000000..68e9596 --- /dev/null +++ b/app/src/main/java/io/aatricks/novelscraper/ui/components/ExternalUrlConfirmationDialog.kt @@ -0,0 +1,43 @@ +package io.aatricks.novelscraper.ui.components + +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp + +@Composable +fun ExternalUrlConfirmationDialog( + url: String, + onConfirm: () -> Unit, + onCancel: () -> Unit +) { + AlertDialog( + onDismissRequest = onCancel, + title = { Text("Open External Link?") }, + text = { + Column { + Text("This link was opened from another application:") + Spacer(Modifier.height(8.dp)) + Text(url, style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant) + Spacer(Modifier.height(16.dp)) + Text("Do you want to open it in Reader? This will not be automatically saved to your library.", style = MaterialTheme.typography.bodyMedium) + } + }, + confirmButton = { + TextButton(onClick = onConfirm) { + Text("Open") + } + }, + dismissButton = { + TextButton(onClick = onCancel) { + Text("Cancel") + } + } + ) +} diff --git a/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt b/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt index 4e6ab02..796831c 100644 --- a/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt +++ b/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt @@ -225,6 +225,7 @@ fun ReaderScreen( sheetState = bottomSheetState ) } + } @Composable diff --git a/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/ReaderViewModel.kt b/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/ReaderViewModel.kt index 66d444f..0e6aea1 100644 --- a/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/ReaderViewModel.kt +++ b/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/ReaderViewModel.kt @@ -9,6 +9,8 @@ import io.aatricks.novelscraper.data.repository.ContentRepository import io.aatricks.novelscraper.data.repository.ExploreRepository import io.aatricks.novelscraper.data.repository.LibraryRepository import io.aatricks.novelscraper.util.TextUtils +import io.aatricks.novelscraper.util.UrlSecurity +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.* @@ -105,9 +107,31 @@ class ReaderViewModel @Inject constructor( val fontFamily: String = "Default", val margins: Int = 16, val paragraphSpacing: Float = 1.0f, - val readerTheme: ReaderTheme = ReaderTheme.DARK + val readerTheme: ReaderTheme = ReaderTheme.DARK, + val pendingExternalUrl: String? = null, + val showExternalUrlConfirmation: Boolean = false ) + fun requestOpenUrl(url: String): Unit { + viewModelScope.launch { + if (UrlSecurity.isSafeUrl(url)) { + updateState { it.copy(pendingExternalUrl = url, showExternalUrlConfirmation = true) } + } else { + updateState { it.copy(toastMessage = "Blocked unsafe or invalid URL") } + } + } + } + + fun confirmExternalUrl(): Unit { + val url = _uiState.value.pendingExternalUrl ?: return + updateState { it.copy(pendingExternalUrl = null, showExternalUrlConfirmation = false) } + loadContent(url) + } + + fun cancelExternalUrl(): Unit { + updateState { it.copy(pendingExternalUrl = null, showExternalUrlConfirmation = false) } + } + fun updateFontSize(newSize: Float): Unit { val size = newSize.coerceIn(12f, 32f) preferencesManager.fontSize = size diff --git a/app/src/main/java/io/aatricks/novelscraper/util/UrlSecurity.kt b/app/src/main/java/io/aatricks/novelscraper/util/UrlSecurity.kt new file mode 100644 index 0000000..067a3f9 --- /dev/null +++ b/app/src/main/java/io/aatricks/novelscraper/util/UrlSecurity.kt @@ -0,0 +1,40 @@ +package io.aatricks.novelscraper.util + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import java.net.InetAddress +import java.net.URL + +object UrlSecurity { + + suspend fun isSafeUrl(url: String): Boolean = withContext(Dispatchers.IO) { + runCatching { + val uri = URL(url) + + // 1. Validate Scheme + if (uri.protocol != "http" && uri.protocol != "https") { + return@runCatching false + } + + val host = uri.host ?: return@runCatching false + + // 2. Check for private IPs / Localhost + // resolving the host to check if it points to a local address + val address = InetAddress.getByName(host) + + when { + address.isLoopbackAddress -> false + address.isSiteLocalAddress -> false + address.isLinkLocalAddress -> false + address.isAnyLocalAddress -> false + // Check for 192.168.x.x, 10.x.x.x, 172.16.x.x - 172.31.x.x + // isSiteLocalAddress covers 192.168.x.x, 10.x.x.x and 172.16.x.x range + else -> true + } + }.getOrElse { + // If the URL is malformed or host cannot be resolved, we treat it as unsafe + // for the purpose of an external intent. + false + } + } +} diff --git a/app/src/test/java/io/aatricks/novelscraper/util/UrlSecurityTest.kt b/app/src/test/java/io/aatricks/novelscraper/util/UrlSecurityTest.kt new file mode 100644 index 0000000..1a049ec --- /dev/null +++ b/app/src/test/java/io/aatricks/novelscraper/util/UrlSecurityTest.kt @@ -0,0 +1,37 @@ +package io.aatricks.novelscraper.util + +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class UrlSecurityTest { + + @Test + fun `isSafeUrl rejects non-http schemes`() = runTest { + assertFalse(UrlSecurity.isSafeUrl("ftp://google.com")) + assertFalse(UrlSecurity.isSafeUrl("file:///etc/passwd")) + assertFalse(UrlSecurity.isSafeUrl("content://provider")) + assertFalse(UrlSecurity.isSafeUrl("javascript:alert(1)")) + } + + @Test + fun `isSafeUrl rejects loopback addresses`() = runTest { + assertFalse(UrlSecurity.isSafeUrl("http://127.0.0.1")) + // localhost might be flaky if no /etc/hosts, but usually works + // assertFalse(UrlSecurity.isSafeUrl("http://localhost")) + } + + @Test + fun `isSafeUrl rejects private addresses`() = runTest { + assertFalse(UrlSecurity.isSafeUrl("http://192.168.1.1")) + assertFalse(UrlSecurity.isSafeUrl("http://10.0.0.1")) + assertFalse(UrlSecurity.isSafeUrl("http://172.16.0.1")) + } + + @Test + fun `isSafeUrl accepts public addresses`() = runTest { + // 8.8.8.8 is Google DNS, definitely public + assertTrue(UrlSecurity.isSafeUrl("http://8.8.8.8")) + } +}