Skip to content

Commit d93efe5

Browse files
committed
fix(url): match OpenPhish entries by the link's full host
Remote lookups reduce a link to its registrable domain so nothing more leaves the phone, and OpenPhish got the same URL although it matches a feed downloaded to the phone. A feed entry on login.example.com couldn't match a link on that host, and matching the base domain would have flagged its siblings. An adapter now says whether it matches on the device; OpenPhish does, and gets the link's scheme and full host (path, query, fragment and credentials still stripped). It flags the entry's host and anything under it, not siblings, and an entry on a whole domain still covers its subdomains. PhishTank still receives only the base domain.
1 parent ce364b2 commit d93efe5

6 files changed

Lines changed: 88 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ All notable changes to CallShield will be documented in this file.
44

55
## Unreleased
66

7+
- A link in a text is checked against OpenPhish by its full host, so a phishing
8+
page on one subdomain is caught without flagging its sibling subdomains.
9+
OpenPhish is matched on the phone. PhishTank still receives only the site's
10+
base domain.
711
- Number details, Lookup and the block explanation describe FCC and FTC
812
complaints in words, one line per kind, instead of the importer's field names
913
("FCC callback_business") and the same complaint repeated.

‎app/src/main/java/com/sysadmindoc/callshield/data/remote/UrlSafetyChecker.kt‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ object UrlSafetyChecker {
8585
verdict = UrlThreatVerdict.UNKNOWN,
8686
)
8787
}
88-
val evidence = lookupUrlEvidence(lookupUrl)
88+
val evidence = lookupUrlEvidence(lookupUrl, normalizeOnDeviceLookupUrl(url).ifBlank { lookupUrl })
8989
evidence.firstOrNull(UrlThreatResult::isMalicious)?.let(::toCheckResult)
9090
?: evidence.firstOrNull()?.let(::toCheckResult)
9191
?: UrlCheckResult(
@@ -99,17 +99,21 @@ object UrlSafetyChecker {
9999
internal suspend fun checkUrlMatches(url: String): List<UrlCheckResult> {
100100
val lookupUrl = normalizeRemoteLookupUrl(url)
101101
if (lookupUrl.isBlank()) return emptyList()
102-
return lookupUrlEvidence(lookupUrl)
102+
return lookupUrlEvidence(lookupUrl, normalizeOnDeviceLookupUrl(url).ifBlank { lookupUrl })
103103
.filter(UrlThreatResult::isMalicious)
104104
.map(::toCheckResult)
105105
}
106106

107-
private suspend fun lookupUrlEvidence(lookupUrl: String): List<UrlThreatResult> {
107+
private suspend fun lookupUrlEvidence(
108+
remoteLookupUrl: String,
109+
onDeviceLookupUrl: String = remoteLookupUrl,
110+
): List<UrlThreatResult> {
108111
if (enabledAdapters.isEmpty()) return emptyList()
109112
val nowMillis = System.currentTimeMillis()
110113
return coroutineScope {
111114
enabledAdapters
112115
.map { adapter ->
116+
val lookupUrl = if (adapter.matchesOnDevice) onDeviceLookupUrl else remoteLookupUrl
113117
async {
114118
val cached = threatCache.get(adapter.source, adapter.sourceVersion, lookupUrl)
115119
if (cached != null) {
@@ -216,6 +220,23 @@ object UrlSafetyChecker {
216220
.toString()
217221
}
218222

223+
/**
224+
* The link's scheme and full host for a list matched on the phone. The path,
225+
* query, fragment and any user name or password still go.
226+
*/
227+
internal fun normalizeOnDeviceLookupUrl(rawUrl: String): String {
228+
val parsedUrl = normalizeCandidateUrl(rawUrl).toHttpUrlOrNull() ?: return ""
229+
return parsedUrl
230+
.newBuilder()
231+
.username("")
232+
.password("")
233+
.encodedPath("/")
234+
.query(null)
235+
.fragment(null)
236+
.build()
237+
.toString()
238+
}
239+
219240
internal fun clearThreatCacheForTests() {
220241
threatCache.clear()
221242
}

‎app/src/main/java/com/sysadmindoc/callshield/data/remote/UrlThreatAdapters.kt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ internal class OpenPhishThreatAdapter(
141141
override val source: UrlThreatSource = UrlThreatSource.OPENPHISH
142142
override val sourceVersion: String = source.defaultVersion
143143
override val isConfigured: Boolean = true
144+
override val matchesOnDevice: Boolean = true
144145

145146
@Volatile
146147
private var feedSnapshot: FeedSnapshot? = null
@@ -161,7 +162,9 @@ internal class OpenPhishThreatAdapter(
161162
UrlThreatCache.DEFAULT_TTL_MILLIS,
162163
(snapshot.expiresAtMillis - nowMillis).coerceAtLeast(1L),
163164
)
164-
return if (host != null && host in snapshot.hosts) {
165+
// A feed entry names the host a phishing page sits on. It flags that host
166+
// and anything under it, but not a sibling on the same domain.
167+
return if (host != null && host.parentHosts().any { it in snapshot.hosts }) {
165168
UrlThreatResult.malicious(
166169
source = source,
167170
sourceVersion = sourceVersion,
@@ -226,6 +229,9 @@ internal class OpenPhishThreatAdapter(
226229
}
227230
}
228231

232+
/** This host and each parent domain of it, the host first. */
233+
private fun String.parentHosts(): Sequence<String> = generateSequence(this) { it.substringAfter('.', "").ifEmpty { null } }
234+
229235
private data class FeedSnapshot(
230236
val hosts: Set<String>,
231237
val expiresAtMillis: Long,

‎app/src/main/java/com/sysadmindoc/callshield/data/remote/UrlThreatModels.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ internal interface UrlThreatAdapter {
104104
val sourceVersion: String
105105
val isConfigured: Boolean
106106

107+
/**
108+
* True for an adapter that matches against a list downloaded to the phone.
109+
* Nothing about the link leaves the phone, so it gets the link's full host;
110+
* a remote service gets only the registrable domain.
111+
*/
112+
val matchesOnDevice: Boolean get() = false
113+
107114
suspend fun lookup(
108115
canonicalUrl: String,
109116
nowMillis: Long = System.currentTimeMillis(),

‎app/src/test/java/com/sysadmindoc/callshield/data/remote/OpenPhishThreatAdapterTest.kt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,41 @@ import org.junit.Assert.assertEquals
1010
import org.junit.Test
1111

1212
class OpenPhishThreatAdapterTest {
13+
private fun feedOf(vararg urls: String): OkHttpClient =
14+
OkHttpClient
15+
.Builder()
16+
.addInterceptor { chain ->
17+
Response
18+
.Builder()
19+
.request(chain.request())
20+
.protocol(Protocol.HTTP_1_1)
21+
.code(200)
22+
.message("OK")
23+
.body(urls.joinToString("\n").toResponseBody("text/plain".toMediaType()))
24+
.build()
25+
}.build()
26+
27+
@Test
28+
fun `a feed entry on a subdomain flags that host and below it, not its siblings`() =
29+
runBlocking {
30+
// Links used to reach the adapter as their registrable domain, so a
31+
// login.example.test entry never matched a link on login.example.test.
32+
val adapter = OpenPhishThreatAdapter(feedOf("https://login.example.test/verify", "https://evil.test/"))
33+
34+
assertEquals(UrlThreatVerdict.MALICIOUS, adapter.lookup("https://login.example.test/", 0L).verdict)
35+
assertEquals(UrlThreatVerdict.MALICIOUS, adapter.lookup("https://eu.login.example.test/", 0L).verdict)
36+
assertEquals(UrlThreatVerdict.CLEAN, adapter.lookup("https://shop.example.test/", 0L).verdict)
37+
assertEquals(UrlThreatVerdict.CLEAN, adapter.lookup("https://example.test/", 0L).verdict)
38+
// An entry on a whole domain still covers its subdomains.
39+
assertEquals(UrlThreatVerdict.MALICIOUS, adapter.lookup("https://pay.evil.test/", 0L).verdict)
40+
}
41+
42+
@Test
43+
fun `only the on-device feed asks for the full host`() {
44+
assertEquals(true, OpenPhishThreatAdapter(feedOf()).matchesOnDevice)
45+
assertEquals(false, PhishTankThreatAdapter().matchesOnDevice)
46+
}
47+
1348
@Test
1449
fun `feed waits six hours and a 304 renews the existing hosts`() =
1550
runBlocking {

‎app/src/test/java/com/sysadmindoc/callshield/data/remote/UrlSafetyCheckerTest.kt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,17 @@ class UrlSafetyCheckerTest {
9797
assertEquals("https://example.test/", normalized)
9898
}
9999

100+
@Test
101+
fun `an on-device list gets the full host and nothing else`() {
102+
// OpenPhish is matched on the phone, so it can see the subdomain its feed
103+
// entries name. The path, query, fragment and credentials still go.
104+
assertEquals(
105+
"https://login.example.test/",
106+
UrlSafetyChecker.normalizeOnDeviceLookupUrl("https://alice:secret@login.example.test/pay?token=secret#frag"),
107+
)
108+
assertEquals("", UrlSafetyChecker.normalizeOnDeviceLookupUrl("not a link"))
109+
}
110+
100111
@Test
101112
fun `remote lookup fails closed when registrable domain cannot be resolved`() {
102113
val normalized =

0 commit comments

Comments
 (0)