From 48219b385ea8f95825d5596cd53d183d760e3827 Mon Sep 17 00:00:00 2001 From: cogwheel Date: Tue, 29 Sep 2026 13:55:44 +0530 Subject: [PATCH] Keep search-result redirects on the original site Redirect hops were compared pairwise and could climb to a parent domain, so victim.github.io could reach attacker.github.io by way of github.io. Every hop is now judged against the original result and may only stay on its host, www twin or subdomains. --- .../services/on_device_web_tools.dart | 23 +++++++++++-------- .../web_search/on_device_web_tools_test.dart | 18 +++++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/packages/conduit_core/lib/features/web_search/services/on_device_web_tools.dart b/packages/conduit_core/lib/features/web_search/services/on_device_web_tools.dart index 1f13e01a1..e7d38c95b 100644 --- a/packages/conduit_core/lib/features/web_search/services/on_device_web_tools.dart +++ b/packages/conduit_core/lib/features/web_search/services/on_device_web_tools.dart @@ -238,8 +238,12 @@ final class OnDeviceWebToolSession { acceptLanguage: region.acceptLanguage, cancel: _cancel, // A search result may only redirect within its own site, so a result - // can't hand the fetch to an unrelated host. - allowRedirect: _userUrls.contains(url) ? null : isSameSiteRedirect, + // can't hand the fetch to an unrelated host. Every hop is judged + // against the result itself, so hops can't chain across a shared + // parent domain. + allowRedirect: _userUrls.contains(url) + ? null + : (_, to) => isSameSiteRedirect(Uri.parse(url), to), ); final extracted = extractReadableText(page); final truncated = @@ -278,18 +282,19 @@ final class OnDeviceWebToolSession { } } -/// Whether a redirect stays on the same site: the same host, its `www` -/// twin, or a subdomain in either direction (`http→https` and path changes -/// are fine). -bool isSameSiteRedirect(Uri from, Uri to) { +/// Whether a redirect from [origin] stays on its site: the same host, its +/// `www` twin, or one of its subdomains (`http→https` and path changes are +/// fine). Never a parent domain: `victim.github.io` must not reach +/// `github.io`, whose other subdomains belong to other people. +bool isSameSiteRedirect(Uri origin, Uri to) { String bare(Uri uri) { final host = uri.host.toLowerCase(); return host.startsWith('www.') ? host.substring(4) : host; } - final a = bare(from); - final b = bare(to); - return a == b || a.endsWith('.$b') || b.endsWith('.$a'); + final site = bare(origin); + final target = bare(to); + return target == site || target.endsWith('.$site'); } /// `http(s)` links in [text], e.g. the user's message, for the fetch diff --git a/packages/conduit_core/test/features/web_search/on_device_web_tools_test.dart b/packages/conduit_core/test/features/web_search/on_device_web_tools_test.dart index abb42f124..f0734e06c 100644 --- a/packages/conduit_core/test/features/web_search/on_device_web_tools_test.dart +++ b/packages/conduit_core/test/features/web_search/on_device_web_tools_test.dart @@ -181,6 +181,24 @@ void main() { } }); + test('a redirect never climbs to a shared parent domain', () { + // Hops are judged against the original result, so victim.github.io + // can't reach attacker.github.io by way of github.io. + final origin = Uri.parse('https://victim.github.io/page'); + expect( + isSameSiteRedirect(origin, Uri.parse('https://github.io/')), + isFalse, + ); + expect( + isSameSiteRedirect(origin, Uri.parse('https://attacker.github.io/')), + isFalse, + ); + expect( + isSameSiteRedirect(origin, Uri.parse('https://docs.victim.github.io/')), + isTrue, + ); + }); + test('links the user wrote are read as written', () { expect( extractWebUrls(