From 303c117eb9e6dfcbab2bab5fe3a3a34d843bf7b7 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Sat, 22 Aug 2026 00:54:44 +0000 Subject: Harden extraction WebView, cache Readability.js, sharpen YouTube thumbs - Cache the ~90KB Readability.js asset read (identical every time) instead of a disk read + UTF-8 decode on the main thread for every single extraction. - Lock down allowFileAccess/allowContentAccess/allowUniversalAccess: this WebView runs full JS against any article link, feed URL, or (via ShareReceiverActivity) any URL another app hands it -- no legitimate reason to ever load file://. - hq720 thumbnail instead of hqdefault (a letterboxed 4:3 crop). - Validate frameSrc is actually http(s) before using it verbatim as a fallback link -- it's attacker-controlled article content that only had to match a substring filter, not the strict video-ID regex, so a crafted file://, content://, or intent:// src couldn't ride along into an ACTION_VIEW intent or a JS-enabled WebView.loadUrl. --- .../feedreader/readability/ReaderExtractor.kt | 39 +++++++++++++++++++--- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/terst/feedreader/readability/ReaderExtractor.kt b/app/src/main/java/org/terst/feedreader/readability/ReaderExtractor.kt index 1edef0e..03a7d7c 100644 --- a/app/src/main/java/org/terst/feedreader/readability/ReaderExtractor.kt +++ b/app/src/main/java/org/terst/feedreader/readability/ReaderExtractor.kt @@ -40,6 +40,18 @@ object ReaderExtractor { private const val TIMEOUT_MS = 25_000L private val json = Json { ignoreUnknownKeys = true } + // The vendored asset (~90KB) is identical every time regardless of which Context reads + // it -- caching it once avoids a disk read + UTF-8 decode on the MAIN thread (this + // whole class runs its WebView work on Dispatchers.Main) on every single extraction, + // which adds up over a reading session that opens/re-extracts many articles. No + // synchronization needed: extractOnMainThread only ever runs on Dispatchers.Main, so + // this is never accessed from more than one thread. + private var cachedReadabilityJs: String? = null + + private fun loadReadabilityJs(context: Context): String = + cachedReadabilityJs ?: context.assets.open("Readability.js").bufferedReader().use { it.readText() } + .also { cachedReadabilityJs = it } + suspend fun extract(context: Context, url: String): ExtractedArticle? = withTimeoutOrNull(TIMEOUT_MS) { withContext(Dispatchers.Main) { @@ -52,6 +64,15 @@ object ReaderExtractor { suspendCancellableCoroutine { continuation -> val webView = WebView(context) webView.settings.javaScriptEnabled = true + // This WebView runs full JS against whatever URL it's given -- every article + // link, every feed URL, and (via ShareReceiverActivity) any URL another + // installed app hands this one. It has no legitimate reason to ever load a + // file:// URL, so don't leave that door open for a malicious page to redirect + // through (WebView defaults allowFileAccess to true pre-API 30). + webView.settings.allowFileAccess = false + webView.settings.allowContentAccess = false + webView.settings.allowFileAccessFromFileURLs = false + webView.settings.allowUniversalAccessFromFileURLs = false // A slow-loading page's evaluateJavascript callback can arrive right as the // outer withTimeoutOrNull cancels this coroutine, racing finish() (JS-callback @@ -70,7 +91,7 @@ object ReaderExtractor { webView.webViewClient = object : WebViewClient() { override fun onPageFinished(view: WebView, finishedUrl: String) { - val readabilityJs = context.assets.open("Readability.js").bufferedReader().use { it.readText() } + val readabilityJs = loadReadabilityJs(context) val script = """ (function() { try { @@ -231,7 +252,11 @@ object ReaderExtractor { var videoId = match[1]; link.href = 'https://www.youtube.com/watch?v=' + videoId; var thumb = document.createElement('img'); - thumb.src = 'https://i.ytimg.com/vi/' + videoId + '/hqdefault.jpg'; + // hq720 (true 16:9 crop) beats hqdefault (a 480x360 4:3 + // crop that letterboxes any widescreen video with black + // bars) and, unlike maxresdefault, is reliably present for + // essentially every video -- no existence check needed. + thumb.src = 'https://i.ytimg.com/vi/' + videoId + '/hq720.jpg'; thumb.alt = 'YouTube video'; link.appendChild(thumb); var caption = document.createElement('figcaption'); @@ -240,8 +265,14 @@ object ReaderExtractor { } else { // No confident single video ID (a playlist, a live stream, or // lazy-load resolution above still came up empty) -- a plain - // link beats a broken thumbnail or a dead iframe. - link.href = frameSrc || 'https://www.youtube.com/'; + // link beats a broken thumbnail or a dead iframe. frameSrc is + // attacker-controlled article content though (it only needed to + // match the querySelector's substring src filter, not the strict + // video-ID regex) -- only use it verbatim if it's actually an + // http(s) URL, so a crafted file://, content://, or intent:// + // iframe src can't ride along into this href and downstream into + // an ACTION_VIEW intent or a JS-enabled WebView.loadUrl. + link.href = /^https?:\/\//i.test(frameSrc) ? frameSrc : 'https://www.youtube.com/'; link.textContent = 'Watch on YouTube'; } frame.parentNode.replaceChild(figure, frame); -- cgit v1.2.3