From 27563e101c24342b36a5a754041bc944d6a274c9 Mon Sep 17 00:00:00 2001 From: ryjohn Date: Wed, 4 Nov 2026 21:06:25 +0000 Subject: [PATCH 1/3] Validate incoming requests to DevTools server The DevTools server (`server_api.dart` now has a `_validateRequest` function that performs a variety of checks to prevent cross-site request forgery and DNS rebinding: * Prevents external websites from triggering local operations through cross-origin `GET` requests. Requests to the DevTools server are now required to be `POST` or `OPTIONS`. Web browsers will allow cross-origin `GET` requests through `img` tags or the `fetch()` API. Malicious websites could call `api/notifyForVmServiceConnection` and connect to a remote VM service, which could lead to CSRF attacks where DTD authentication tokens are read through `/api/getDtdUri`.  * Prevents DNS rebinding attacks by validating that the `host` header is within an allowlist of loopback addresses (`localhost`, `127.0.0.1`, etc) and is not malformed to mitigate DNS rebinding. * Rejects requests where the `Sec-Fetch-Site` header is set to `cross-site` unless the `Origin` header is a valid loopback address. Otherwise, only `same-origin`, `same-site`, or `none` are allowed. --- .../lib/src/shared/server/server.dart | 5 +- .../devtools_app/web/flutter_bootstrap.js | 7 +- packages/devtools_shared/CHANGELOG.md | 4 + .../devtools_shared/lib/src/devtools_api.dart | 8 + .../lib/src/server/server_api.dart | 109 +++++++ .../test/server/deeplink_api_test.dart | 10 +- .../test/server/dtd_api_test.dart | 2 +- .../test/server/general_api_test.dart | 279 +++++++++++++++++- 8 files changed, 415 insertions(+), 9 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/server/server.dart b/packages/devtools_app/lib/src/shared/server/server.dart index 8b40cbbf2da..61aaf1ad7b7 100644 --- a/packages/devtools_app/lib/src/shared/server/server.dart +++ b/packages/devtools_app/lib/src/shared/server/server.dart @@ -99,7 +99,10 @@ Future request(String url) async { Response? response; try { - response = await post(buildDevToolsServerRequestUri(url)); + response = await post( + buildDevToolsServerRequestUri(url), + headers: const {devToolsRequestHeader: '1'}, + ); } catch (_) {} return response; diff --git a/packages/devtools_app/web/flutter_bootstrap.js b/packages/devtools_app/web/flutter_bootstrap.js index ee6a54d68db..dbe1a2f8918 100644 --- a/packages/devtools_app/web/flutter_bootstrap.js +++ b/packages/devtools_app/web/flutter_bootstrap.js @@ -40,7 +40,12 @@ async function getDevToolsWasmOptOutPreference() { // running from `flutter run` anyway. const request = 'api/getPreferenceValue?key=experiment.wasmOptOut'; try { - const response = await fetch(request); + // This header name must match `devToolsRequestHeader` in + // `packages/devtools_shared/lib/src/devtools_api.dart`. + const response = await fetch(request, { + method: 'POST', + headers: { 'X-DevTools-Request': '1' }, + }); if (!response.ok) { console.warn(`[${response.status} response] ${request}`); return false; diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md index 61d8d705d40..488379c66e3 100644 --- a/packages/devtools_shared/CHANGELOG.md +++ b/packages/devtools_shared/CHANGELOG.md @@ -19,6 +19,10 @@ found in the LICENSE file or at https://developers.google.com/open-source/licens `DASH__SUPPRESS_ANALYTICS=true` is now passed on instead of being reset to `false`. * Require `package:unified_analytics` `^8.4.0`. +* Require `POST` (or `OPTIONS` CORS preflight) requests and validate `Host`, + `Origin`, and `Sec-Fetch-Site` headers in `ServerApi.handle` to prevent + cross-site and DNS rebinding requests. +* Add `devToolsRequestHeader` constant (`X-DevTools-Request`). # 14.0.0 diff --git a/packages/devtools_shared/lib/src/devtools_api.dart b/packages/devtools_shared/lib/src/devtools_api.dart index fd2ca9338ec..c3d9a7e393f 100644 --- a/packages/devtools_shared/lib/src/devtools_api.dart +++ b/packages/devtools_shared/lib/src/devtools_api.dart @@ -5,6 +5,14 @@ /// All server APIs prefix: const apiPrefix = 'api/'; +/// Custom HTTP header included on DevTools server API requests to force a CORS +/// preflight `OPTIONS` request in browsers and prevent simple cross-origin +/// `POST` requests. +/// +/// This value must match the header sent in +/// `packages/devtools_app/web/flutter_bootstrap.js`. +const devToolsRequestHeader = 'X-DevTools-Request'; + /// Key used for any request or response to specify a value argument. const apiParameterValueKey = 'value'; diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index b4d6b7e8d93..d974c263cec 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -44,11 +44,102 @@ class ServerApi { static const logsKey = 'logs'; static const errorKey = 'error'; + static const _postMethod = 'POST'; + static const _optionsMethod = 'OPTIONS'; + static const _originHeader = 'origin'; + static const _secFetchSiteHeader = 'sec-fetch-site'; + static const _crossSiteSecFetchSite = 'cross-site'; + static const _allowedSecFetchSites = {'same-origin', 'same-site', 'none'}; + /// Determines whether or not [request] is an API call. static bool canHandle(shelf.Request request) { return request.url.path.startsWith(apiPrefix); } + /// Validates [request] to mitigate CSRF, DNS rebinding, and cross-origin + /// attacks before dispatching to any [ServerApi] handler. + /// + /// Returns a [shelf.Response] if the request is rejected, or `null` if the + /// request is valid. + static shelf.Response? _validateRequest( + shelf.Request request, + ServerApi api, + ) { + final method = request.method.toUpperCase(); + if (method != _postMethod && method != _optionsMethod) { + return shelf.Response( + HttpStatus.methodNotAllowed, + body: 'Method Not Allowed: ${request.method}', + ); + } + + final hostHeaders = request.headersAll[HttpHeaders.hostHeader]; + if (hostHeaders != null && hostHeaders.length > 1) { + return api.forbidden('Invalid Host header'); + } + final hostHeader = request.headers[HttpHeaders.hostHeader]; + final host = hostHeader != null + ? _extractHostFromHeader(hostHeader) + : request.requestedUri.host; + if (host == null || + !_isLoopbackHost(host) || + !_isLoopbackHost(request.requestedUri.host)) { + return api.forbidden('Invalid Host header'); + } + + final originHeaders = request.headersAll[_originHeader]; + if (originHeaders != null && originHeaders.length > 1) { + return api.forbidden('Invalid Origin header'); + } + final originHeader = request.headers[_originHeader]; + if (originHeader != null) { + final originUri = Uri.tryParse(originHeader.trim()); + if (originUri == null || + originUri.userInfo.isNotEmpty || + !_isLoopbackHost(originUri.host)) { + return api.forbidden('Invalid Origin header'); + } + } + + final secFetchSite = request.headers[_secFetchSiteHeader] + ?.trim() + .toLowerCase(); + // Allow `cross-site` when a validated loopback `Origin` header is present + // (e.g. `http://localhost:` calling `http://127.0.0.1:` in + // `debug_devtools_server` mode). + if (secFetchSite != null && + !_allowedSecFetchSites.contains(secFetchSite) && + !(secFetchSite == _crossSiteSecFetchSite && originHeader != null)) { + return api.forbidden('Cross-site requests are not allowed'); + } + + return null; + } + + /// Extracts the host component from an HTTP `Host` header value, or returns + /// `null` if the header is malformed. + static String? _extractHostFromHeader(String hostHeader) { + final trimmed = hostHeader.trim(); + if (trimmed.isEmpty) return null; + if (trimmed == '::1') return '::1'; + final parsed = Uri.tryParse('http://$trimmed'); + if (parsed == null || + parsed.userInfo.isNotEmpty || + parsed.path.isNotEmpty || + parsed.hasQuery || + parsed.hasFragment || + parsed.host.isEmpty) { + return null; + } + return parsed.host; + } + + /// Returns whether [host] is a loopback host + /// (`localhost`, `127.x.x.x`, or `::1`). + static bool _isLoopbackHost(String host) => + host == 'localhost' || + (InternetAddress.tryParse(host)?.isLoopback ?? false); + /// Handles all requests. /// /// To override an API call, pass in a subclass of [ServerApi]. @@ -60,6 +151,24 @@ class ServerApi { DtdInfo? dtd, }) { api ??= ServerApi(); + final validationErrorResponse = _validateRequest(request, api); + if (validationErrorResponse != null) { + return validationErrorResponse; + } + if (request.method.toUpperCase() == _optionsMethod) { + // Respond to CORS preflight requests (e.g. from `debug_devtools_server` + // running on another loopback port). `Access-Control-Allow-Origin` is + // intentionally omitted here and controlled by the server's `--disable-cors` + // configuration. + return shelf.Response( + HttpStatus.noContent, + headers: const { + HttpHeaders.accessControlAllowMethodsHeader: + '$_postMethod, $_optionsMethod', + HttpHeaders.accessControlAllowHeadersHeader: devToolsRequestHeader, + }, + ); + } final queryParams = request.requestedUri.queryParameters; switch (request.url.path) { case apiNotifyForVmServiceConnection: diff --git a/packages/devtools_shared/test/server/deeplink_api_test.dart b/packages/devtools_shared/test/server/deeplink_api_test.dart index e3d39f26dee..af2b1ddf9ea 100644 --- a/packages/devtools_shared/test/server/deeplink_api_test.dart +++ b/packages/devtools_shared/test/server/deeplink_api_test.dart @@ -18,7 +18,7 @@ void main() { test('handle deeplink api ${DeeplinkApi.androidBuildVariants}', () async { const expectedRootPath = '/abc'; final request = Request( - 'get', + 'post', Uri( scheme: 'https', host: 'localhost', @@ -46,7 +46,7 @@ void main() { 'handle deeplink api ${DeeplinkApi.androidBuildVariants} returns bad request if no root path', () async { final request = Request( - 'get', + 'post', Uri( scheme: 'https', host: 'localhost', @@ -67,7 +67,7 @@ void main() { const buildVariant = 'buildVariant'; const someMessage = 'some message'; final request = Request( - 'get', + 'post', Uri( scheme: 'https', host: 'localhost', @@ -97,7 +97,7 @@ void main() { const expectedRootPath = '/abc'; const someMessage = 'some message'; final request = Request( - 'get', + 'post', Uri( scheme: 'https', host: 'localhost', @@ -129,7 +129,7 @@ void main() { const target = 'target'; const someMessage = 'some message'; final request = Request( - 'get', + 'post', Uri( scheme: 'https', host: 'localhost', diff --git a/packages/devtools_shared/test/server/dtd_api_test.dart b/packages/devtools_shared/test/server/dtd_api_test.dart index e6dd18d13b4..6e1cc454cc3 100644 --- a/packages/devtools_shared/test/server/dtd_api_test.dart +++ b/packages/devtools_shared/test/server/dtd_api_test.dart @@ -18,7 +18,7 @@ void main() { test('handle ${DtdApi.apiGetDtdUri} succeeds', () async { final dtdUri = Uri.parse('ws://dtd/uri'); final request = Request( - 'get', + 'post', Uri(scheme: 'https', host: 'localhost', path: DtdApi.apiGetDtdUri), ); final response = await ServerApi.handle( diff --git a/packages/devtools_shared/test/server/general_api_test.dart b/packages/devtools_shared/test/server/general_api_test.dart index 0a9fad7f616..7d3bcd0a3e1 100644 --- a/packages/devtools_shared/test/server/general_api_test.dart +++ b/packages/devtools_shared/test/server/general_api_test.dart @@ -17,6 +17,283 @@ import '../helpers/helpers.dart'; void main() { group('General DevTools server API', () { + group('request validation', () { + Future sendServerRequest({ + String method = 'POST', + String uriHost = 'localhost', + Map? headers, + // ignore: avoid-redundant-async, returning FutureOr. + }) async { + final request = Request( + method, + Uri(scheme: 'http', host: uriHost, path: DtdApi.apiGetDtdUri), + headers: headers, + ); + return server.ServerApi.handle( + request, + extensionsManager: ExtensionsManager(), + deeplinkManager: FakeDeeplinkManager(), + ); + } + + test( + 'rejects unsupported HTTP methods with 405 Method Not Allowed', + () async { + for (final method in [ + 'GET', + 'get', + 'PUT', + 'DELETE', + 'PATCH', + 'HEAD', + ]) { + final response = await sendServerRequest(method: method); + expect(response.statusCode, HttpStatus.methodNotAllowed); + expect( + await response.readAsString(), + 'Method Not Allowed: $method', + ); + } + }, + ); + + test( + 'handles OPTIONS preflight requests from loopback origins', + () async { + for (final method in ['OPTIONS', 'options']) { + for (final headers in [ + { + HttpHeaders.hostHeader: '127.0.0.1:9104', + 'origin': 'http://localhost:54321', + 'sec-fetch-site': 'cross-site', + HttpHeaders.accessControlRequestMethodHeader: 'POST', + HttpHeaders.accessControlRequestHeadersHeader: + devToolsRequestHeader.toLowerCase(), + }, + { + HttpHeaders.hostHeader: 'localhost:9104', + 'origin': 'http://localhost:54321', + 'sec-fetch-site': 'same-site', + HttpHeaders.accessControlRequestMethodHeader: 'POST', + HttpHeaders.accessControlRequestHeadersHeader: + devToolsRequestHeader.toLowerCase(), + }, + ]) { + final response = await sendServerRequest( + method: method, + headers: headers, + ); + expect(response.statusCode, HttpStatus.noContent); + expect(await response.readAsString(), isEmpty); + expect( + response.headers[HttpHeaders.accessControlAllowMethodsHeader], + 'POST, OPTIONS', + ); + expect( + response.headers[HttpHeaders.accessControlAllowHeadersHeader], + devToolsRequestHeader, + ); + expect( + response.headers.containsKey( + HttpHeaders.accessControlAllowOriginHeader, + ), + isFalse, + ); + } + } + }, + ); + + test('rejects OPTIONS preflight requests with invalid headers', () async { + final invalidPreflights = [ + { + HttpHeaders.hostHeader: 'evil.com:9104', + 'origin': 'http://localhost:54321', + }, + { + HttpHeaders.hostHeader: '127.0.0.1:9104', + 'origin': 'https://evil.com', + 'sec-fetch-site': 'cross-site', + }, + { + HttpHeaders.hostHeader: '127.0.0.1:9104', + 'sec-fetch-site': 'cross-site', + }, + ]; + for (final headers in invalidPreflights) { + final response = await sendServerRequest( + method: 'OPTIONS', + headers: headers, + ); + expect(response.statusCode, HttpStatus.forbidden); + expect( + response.headers.containsKey( + HttpHeaders.accessControlAllowMethodsHeader, + ), + isFalse, + ); + expect( + response.headers.containsKey( + HttpHeaders.accessControlAllowHeadersHeader, + ), + isFalse, + ); + } + }); + + test('rejects non-loopback Host headers with 403 Forbidden', () async { + const invalidHosts = [ + 'evil.com', + 'evil.com:9100', + 'localhost.evil.com', + '127.0.0.1.evil.com', + 'localhost@evil.com', + '8.8.8.8', + '0.0.0.0', + '', + ]; + for (final invalidHost in invalidHosts) { + final response = await sendServerRequest( + headers: {HttpHeaders.hostHeader: invalidHost}, + ); + expect( + response.statusCode, + HttpStatus.forbidden, + reason: 'Expected Host "$invalidHost" to be rejected', + ); + expect(await response.readAsString(), 'Invalid Host header'); + } + + final nonLoopbackUriResponse = await sendServerRequest( + uriHost: 'evil.com', + ); + expect(nonLoopbackUriResponse.statusCode, HttpStatus.forbidden); + expect( + await nonLoopbackUriResponse.readAsString(), + 'Invalid Host header', + ); + }); + + test('accepts loopback Host headers', () async { + const validHosts = [ + 'localhost', + 'localhost:9100', + '127.0.0.1', + '127.0.0.1:9100', + '127.0.0.2:9100', + '[::1]', + '[::1]:9100', + '::1', + ]; + for (final validHost in validHosts) { + final response = await sendServerRequest( + headers: {HttpHeaders.hostHeader: validHost}, + ); + expect( + response.statusCode, + HttpStatus.ok, + reason: 'Expected Host "$validHost" to be accepted', + ); + } + }); + + test('rejects non-loopback Origin headers with 403 Forbidden', () async { + const invalidOrigins = [ + 'https://evil.com', + 'http://evil.com:9100', + 'http://localhost.evil.com', + 'http://127.0.0.1.evil.com', + 'http://localhost@evil.com', + 'null', + '', + ]; + for (final invalidOrigin in invalidOrigins) { + final response = await sendServerRequest( + headers: {'origin': invalidOrigin}, + ); + expect( + response.statusCode, + HttpStatus.forbidden, + reason: 'Expected Origin "$invalidOrigin" to be rejected', + ); + expect(await response.readAsString(), 'Invalid Origin header'); + } + }); + + test('accepts loopback Origin headers', () async { + const validOrigins = [ + 'http://localhost:9100', + 'https://localhost:9100', + 'http://127.0.0.1:9100', + 'http://127.0.0.2:9100', + 'http://[::1]:9100', + ]; + for (final validOrigin in validOrigins) { + final response = await sendServerRequest( + headers: {'origin': validOrigin}, + ); + expect( + response.statusCode, + HttpStatus.ok, + reason: 'Expected Origin "$validOrigin" to be accepted', + ); + } + }); + + test( + 'rejects Sec-Fetch-Site: cross-site without loopback Origin', + () async { + final response = await sendServerRequest( + headers: {'sec-fetch-site': 'cross-site'}, + ); + expect(response.statusCode, HttpStatus.forbidden); + expect( + await response.readAsString(), + 'Cross-site requests are not allowed', + ); + + final evilOriginResponse = await sendServerRequest( + headers: { + 'origin': 'https://evil.com', + 'sec-fetch-site': 'cross-site', + }, + ); + expect(evilOriginResponse.statusCode, HttpStatus.forbidden); + + final invalidValueWithLoopbackOrigin = await sendServerRequest( + headers: { + 'origin': 'http://localhost:54321', + 'sec-fetch-site': 'unexpected', + }, + ); + expect( + invalidValueWithLoopbackOrigin.statusCode, + HttpStatus.forbidden, + ); + + final crossPortLoopbackResponse = await sendServerRequest( + headers: { + HttpHeaders.hostHeader: '127.0.0.1:9104', + 'origin': 'http://localhost:54321', + 'sec-fetch-site': 'cross-site', + }, + ); + expect(crossPortLoopbackResponse.statusCode, HttpStatus.ok); + + for (final allowed in ['same-origin', 'same-site', 'none']) { + final okResponse = await sendServerRequest( + headers: {'sec-fetch-site': allowed}, + ); + expect( + okResponse.statusCode, + HttpStatus.ok, + reason: 'Expected Sec-Fetch-Site "$allowed" to be accepted', + ); + } + }, + ); + }); + group(apiNotifyForVmServiceConnection, () { Future sendNotifyRequest({ required DtdInfo? dtd, @@ -24,7 +301,7 @@ void main() { // ignore: avoid-redundant-async, returning FutureOr. }) async { final request = Request( - 'get', + 'post', Uri( scheme: 'https', host: 'localhost', From e5f4db87ae34e07677db5cb0e0bdcbbfa1e66176 Mon Sep 17 00:00:00 2001 From: John Ryan Date: Mon, 9 Nov 2026 16:30:14 -0800 Subject: [PATCH 2/3] Add release note for DevTools server request validation --- packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 86b2a491380..fdf1a924773 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -21,6 +21,11 @@ TODO: Remove this section if there are not any updates. screens: colors that are hard to see against the current light or dark theme, such as black or white, are now adjusted to remain legible. [#10060](https://github.com/flutter/devtools/pull/10060) +* Validated incoming DevTools server requests by requiring `POST` or `OPTIONS` + methods and loopback `Host`, `Origin`, and `Sec-Fetch-Site` headers to protect + against cross-site request forgery (CSRF), DNS rebinding, and cross-origin + attacks. + [#10082](https://github.com/flutter/devtools/pull/10082) ## Inspector updates From 267d433db217d833682db83847b61946e2bf80b5 Mon Sep 17 00:00:00 2001 From: John Ryan Date: Tue, 10 Nov 2026 10:56:26 -0800 Subject: [PATCH 3/3] Address review feedback on DevTools server validation Enforce `X-DevTools-Request` on `POST` requests, require `http`/`https` origins without a path or query, disallow `Sec-Fetch-Site: same-site` when `Origin` is absent, and return an `Allow` header on `405` responses. Also add `disableOriginCheck` and `allowedOrigins` parameters to `ServerApi.handle` for SDK flag parity and update the server tests. --- .../release_notes/NEXT_RELEASE_NOTES.md | 8 +- packages/devtools_shared/CHANGELOG.md | 7 +- .../lib/src/server/server_api.dart | 129 ++++++- .../test/server/deeplink_api_test.dart | 5 + .../server/devtools_extensions_api_test.dart | 2 + .../test/server/dtd_api_test.dart | 1 + .../test/server/general_api_test.dart | 318 ++++++++++++++++-- pubspec.lock | 12 +- 8 files changed, 429 insertions(+), 53 deletions(-) diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index fdf1a924773..3097a4a5685 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -21,10 +21,10 @@ TODO: Remove this section if there are not any updates. screens: colors that are hard to see against the current light or dark theme, such as black or white, are now adjusted to remain legible. [#10060](https://github.com/flutter/devtools/pull/10060) -* Validated incoming DevTools server requests by requiring `POST` or `OPTIONS` - methods and loopback `Host`, `Origin`, and `Sec-Fetch-Site` headers to protect - against cross-site request forgery (CSRF), DNS rebinding, and cross-origin - attacks. +* Validated incoming DevTools server requests by requiring `POST` (with + `X-DevTools-Request`) or `OPTIONS` methods and validating `Host`, `Origin`, + and `Sec-Fetch-Site` headers to protect against cross-site request forgery + (CSRF), DNS rebinding, and cross-origin attacks. [#10082](https://github.com/flutter/devtools/pull/10082) ## Inspector updates diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md index 488379c66e3..ce1102d1671 100644 --- a/packages/devtools_shared/CHANGELOG.md +++ b/packages/devtools_shared/CHANGELOG.md @@ -19,9 +19,10 @@ found in the LICENSE file or at https://developers.google.com/open-source/licens `DASH__SUPPRESS_ANALYTICS=true` is now passed on instead of being reset to `false`. * Require `package:unified_analytics` `^8.4.0`. -* Require `POST` (or `OPTIONS` CORS preflight) requests and validate `Host`, - `Origin`, and `Sec-Fetch-Site` headers in `ServerApi.handle` to prevent - cross-site and DNS rebinding requests. +* Require `POST` (with `devToolsRequestHeader`) or `OPTIONS` CORS preflight + requests and validate `Host`, `Origin`, and `Sec-Fetch-Site` headers in + `ServerApi.handle` to prevent cross-site and DNS rebinding requests, with + optional `disableOriginCheck` and `allowedOrigins` parameters. * Add `devToolsRequestHeader` constant (`X-DevTools-Request`). # 14.0.0 diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index d974c263cec..0b37b3feb0b 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -48,8 +48,13 @@ class ServerApi { static const _optionsMethod = 'OPTIONS'; static const _originHeader = 'origin'; static const _secFetchSiteHeader = 'sec-fetch-site'; - static const _crossSiteSecFetchSite = 'cross-site'; - static const _allowedSecFetchSites = {'same-origin', 'same-site', 'none'}; + static const _allowedSecFetchSitesWithoutOrigin = {'same-origin', 'none'}; + static const _allowedSecFetchSitesWithOrigin = { + 'same-origin', + 'same-site', + 'cross-site', + 'none', + }; /// Determines whether or not [request] is an API call. static bool canHandle(shelf.Request request) { @@ -59,20 +64,43 @@ class ServerApi { /// Validates [request] to mitigate CSRF, DNS rebinding, and cross-origin /// attacks before dispatching to any [ServerApi] handler. /// + /// When [disableOriginCheck] is `true`, `Host`, `Origin`, and + /// `Sec-Fetch-Site` validation is skipped, while HTTP method and + /// [devToolsRequestHeader] checks are still enforced. + /// + /// When [disableOriginCheck] is `false`, requests from loopback hosts/origins + /// and any URI in [allowedOrigins] are accepted. + /// /// Returns a [shelf.Response] if the request is rejected, or `null` if the /// request is valid. static shelf.Response? _validateRequest( shelf.Request request, - ServerApi api, - ) { + ServerApi api, { + required bool disableOriginCheck, + required List allowedOrigins, + }) { final method = request.method.toUpperCase(); if (method != _postMethod && method != _optionsMethod) { return shelf.Response( HttpStatus.methodNotAllowed, body: 'Method Not Allowed: ${request.method}', + headers: const { + HttpHeaders.allowHeader: '$_postMethod, $_optionsMethod', + }, ); } + if (method == _postMethod) { + final requestHeader = request.headers[devToolsRequestHeader]; + if (requestHeader == null || requestHeader.trim().isEmpty) { + return api.forbidden('Missing $devToolsRequestHeader header'); + } + } + + if (disableOriginCheck) { + return null; + } + final hostHeaders = request.headersAll[HttpHeaders.hostHeader]; if (hostHeaders != null && hostHeaders.length > 1) { return api.forbidden('Invalid Host header'); @@ -82,8 +110,8 @@ class ServerApi { ? _extractHostFromHeader(hostHeader) : request.requestedUri.host; if (host == null || - !_isLoopbackHost(host) || - !_isLoopbackHost(request.requestedUri.host)) { + !_isAllowedHost(host, allowedOrigins) || + !_isAllowedHost(request.requestedUri.host, allowedOrigins)) { return api.forbidden('Invalid Host header'); } @@ -95,8 +123,13 @@ class ServerApi { if (originHeader != null) { final originUri = Uri.tryParse(originHeader.trim()); if (originUri == null || + !_isHttpOrHttps(originUri) || originUri.userInfo.isNotEmpty || - !_isLoopbackHost(originUri.host)) { + originUri.host.isEmpty || + originUri.path.isNotEmpty || + originUri.hasQuery || + originUri.hasFragment || + !_isAllowedOrigin(originUri, allowedOrigins)) { return api.forbidden('Invalid Origin header'); } } @@ -104,13 +137,13 @@ class ServerApi { final secFetchSite = request.headers[_secFetchSiteHeader] ?.trim() .toLowerCase(); - // Allow `cross-site` when a validated loopback `Origin` header is present - // (e.g. `http://localhost:` calling `http://127.0.0.1:` in - // `debug_devtools_server` mode). - if (secFetchSite != null && - !_allowedSecFetchSites.contains(secFetchSite) && - !(secFetchSite == _crossSiteSecFetchSite && originHeader != null)) { - return api.forbidden('Cross-site requests are not allowed'); + if (secFetchSite != null) { + final allowedSites = originHeader != null + ? _allowedSecFetchSitesWithOrigin + : _allowedSecFetchSitesWithoutOrigin; + if (!allowedSites.contains(secFetchSite)) { + return api.forbidden('Cross-site requests are not allowed'); + } } return null; @@ -134,24 +167,84 @@ class ServerApi { return parsed.host; } + /// Returns whether [uri] uses the `http` or `https` scheme. + static bool _isHttpOrHttps(Uri uri) { + return uri.isScheme('http') || uri.isScheme('https'); + } + + /// Returns whether [host] is a loopback host or matches the host of an entry + /// in [allowedOrigins]. + static bool _isAllowedHost(String host, List allowedOrigins) { + if (host.isEmpty) return false; + if (_isLoopbackHost(host)) return true; + + for (final allowedOrigin in allowedOrigins) { + if (allowedOrigin.host.isEmpty) continue; + if (_hostsMatch(allowedOrigin.host, host)) return true; + } + return false; + } + + /// Returns whether [originUri] is a loopback origin or matches the scheme, + /// host, and port of an HTTP(S) URI in [allowedOrigins]. + static bool _isAllowedOrigin(Uri originUri, List allowedOrigins) { + if (originUri.host.isEmpty) return false; + if (_isLoopbackHost(originUri.host)) return true; + + for (final allowedOrigin in allowedOrigins) { + if (!_isHttpOrHttps(allowedOrigin)) continue; + if (allowedOrigin.host.isEmpty) continue; + if (originUri.scheme != allowedOrigin.scheme) continue; + if (originUri.port != allowedOrigin.port) continue; + if (_hostsMatch(allowedOrigin.host, originUri.host)) return true; + } + return false; + } + + /// Returns whether [a] and [b] refer to the same hostname or IP address. + static bool _hostsMatch(String a, String b) { + if (a.toLowerCase() == b.toLowerCase()) return true; + final addrA = InternetAddress.tryParse(a); + final addrB = InternetAddress.tryParse(b); + if (addrA == null || addrB == null) return false; + return const ListEquality().equals(addrA.rawAddress, addrB.rawAddress); + } + /// Returns whether [host] is a loopback host /// (`localhost`, `127.x.x.x`, or `::1`). - static bool _isLoopbackHost(String host) => - host == 'localhost' || - (InternetAddress.tryParse(host)?.isLoopback ?? false); + static bool _isLoopbackHost(String host) { + if (host.toLowerCase() == 'localhost') return true; + final address = InternetAddress.tryParse(host); + return address?.isLoopback ?? false; + } /// Handles all requests. /// /// To override an API call, pass in a subclass of [ServerApi]. + /// + /// Set [disableOriginCheck] to `true` to bypass `Host`, `Origin`, and + /// `Sec-Fetch-Site` validation while still enforcing HTTP method and + /// [devToolsRequestHeader] checks. + /// + /// Pass [allowedOrigins] to permit requests from specific non-loopback + /// origins (and their corresponding `Host` headers) in addition to loopback + /// hosts and origins. static FutureOr handle( shelf.Request request, { required ExtensionsManager extensionsManager, required DeeplinkManager deeplinkManager, ServerApi? api, DtdInfo? dtd, + bool disableOriginCheck = false, + List allowedOrigins = const [], }) { api ??= ServerApi(); - final validationErrorResponse = _validateRequest(request, api); + final validationErrorResponse = _validateRequest( + request, + api, + disableOriginCheck: disableOriginCheck, + allowedOrigins: allowedOrigins, + ); if (validationErrorResponse != null) { return validationErrorResponse; } diff --git a/packages/devtools_shared/test/server/deeplink_api_test.dart b/packages/devtools_shared/test/server/deeplink_api_test.dart index af2b1ddf9ea..1ce9a91ad4a 100644 --- a/packages/devtools_shared/test/server/deeplink_api_test.dart +++ b/packages/devtools_shared/test/server/deeplink_api_test.dart @@ -27,6 +27,7 @@ void main() { DeeplinkApi.deeplinkRootPathPropertyName: expectedRootPath, }, ), + headers: const {devToolsRequestHeader: '1'}, ); final fakeManager = FakeDeeplinkManager(); fakeManager.responseForGetAndroidBuildVariants = { @@ -52,6 +53,7 @@ void main() { host: 'localhost', path: DeeplinkApi.androidBuildVariants, ), + headers: const {devToolsRequestHeader: '1'}, ); final response = await ServerApi.handle( request, @@ -77,6 +79,7 @@ void main() { DeeplinkApi.androidBuildVariantPropertyName: buildVariant, }, ), + headers: const {devToolsRequestHeader: '1'}, ); final fakeManager = FakeDeeplinkManager(); fakeManager.responseForGetAndroidAppLinkSettings = { @@ -106,6 +109,7 @@ void main() { DeeplinkApi.deeplinkRootPathPropertyName: expectedRootPath, }, ), + headers: const {devToolsRequestHeader: '1'}, ); final fakeManager = FakeDeeplinkManager(); fakeManager.responseForGetIosBuildOptions = { @@ -140,6 +144,7 @@ void main() { DeeplinkApi.xcodeTargetPropertyName: target, }, ), + headers: const {devToolsRequestHeader: '1'}, ); final fakeManager = FakeDeeplinkManager(); fakeManager.responseForGetIosUniversalLinkSettings = { diff --git a/packages/devtools_shared/test/server/devtools_extensions_api_test.dart b/packages/devtools_shared/test/server/devtools_extensions_api_test.dart index ca0d9059dd1..68f366471c5 100644 --- a/packages/devtools_shared/test/server/devtools_extensions_api_test.dart +++ b/packages/devtools_shared/test/server/devtools_extensions_api_test.dart @@ -73,6 +73,7 @@ void main() { : null, }, ), + headers: const {devToolsRequestHeader: '1'}, ); return await ServerApi.handle( request, @@ -214,6 +215,7 @@ void main() { ExtensionsApi.enabledStatePropertyName: enable.toString(), }, ), + headers: const {devToolsRequestHeader: '1'}, ); return await ServerApi.handle( request, diff --git a/packages/devtools_shared/test/server/dtd_api_test.dart b/packages/devtools_shared/test/server/dtd_api_test.dart index 6e1cc454cc3..873692381b1 100644 --- a/packages/devtools_shared/test/server/dtd_api_test.dart +++ b/packages/devtools_shared/test/server/dtd_api_test.dart @@ -20,6 +20,7 @@ void main() { final request = Request( 'post', Uri(scheme: 'https', host: 'localhost', path: DtdApi.apiGetDtdUri), + headers: const {devToolsRequestHeader: '1'}, ); final response = await ServerApi.handle( request, diff --git a/packages/devtools_shared/test/server/general_api_test.dart b/packages/devtools_shared/test/server/general_api_test.dart index 7d3bcd0a3e1..f0902307d63 100644 --- a/packages/devtools_shared/test/server/general_api_test.dart +++ b/packages/devtools_shared/test/server/general_api_test.dart @@ -22,17 +22,27 @@ void main() { String method = 'POST', String uriHost = 'localhost', Map? headers, + bool includeDevToolsRequestHeader = true, + bool disableOriginCheck = false, + List allowedOrigins = const [], // ignore: avoid-redundant-async, returning FutureOr. }) async { + final requestHeaders = { + if (includeDevToolsRequestHeader && method.toUpperCase() == 'POST') + devToolsRequestHeader: '1', + ...?headers, + }; final request = Request( method, Uri(scheme: 'http', host: uriHost, path: DtdApi.apiGetDtdUri), - headers: headers, + headers: requestHeaders, ); return server.ServerApi.handle( request, extensionsManager: ExtensionsManager(), deeplinkManager: FakeDeeplinkManager(), + disableOriginCheck: disableOriginCheck, + allowedOrigins: allowedOrigins, ); } @@ -49,6 +59,7 @@ void main() { ]) { final response = await sendServerRequest(method: method); expect(response.statusCode, HttpStatus.methodNotAllowed); + expect(response.headers[HttpHeaders.allowHeader], 'POST, OPTIONS'); expect( await response.readAsString(), 'Method Not Allowed: $method', @@ -57,6 +68,37 @@ void main() { }, ); + test( + 'rejects POST requests missing or blank $devToolsRequestHeader header', + () async { + final missingResponse = await sendServerRequest( + includeDevToolsRequestHeader: false, + ); + expect(missingResponse.statusCode, HttpStatus.forbidden); + expect( + await missingResponse.readAsString(), + 'Missing $devToolsRequestHeader header', + ); + + for (final blankValue in ['', ' ']) { + final blankResponse = await sendServerRequest( + headers: {devToolsRequestHeader: blankValue}, + ); + expect(blankResponse.statusCode, HttpStatus.forbidden); + expect( + await blankResponse.readAsString(), + 'Missing $devToolsRequestHeader header', + ); + } + + final lowercaseResponse = await sendServerRequest( + includeDevToolsRequestHeader: false, + headers: {devToolsRequestHeader.toLowerCase(): '1'}, + ); + expect(lowercaseResponse.statusCode, HttpStatus.ok); + }, + ); + test( 'handles OPTIONS preflight requests from loopback origins', () async { @@ -172,6 +214,17 @@ void main() { await nonLoopbackUriResponse.readAsString(), 'Invalid Host header', ); + + final duplicateHostResponse = await sendServerRequest( + headers: { + HttpHeaders.hostHeader: ['localhost:9104', 'localhost:9104'], + }, + ); + expect(duplicateHostResponse.statusCode, HttpStatus.forbidden); + expect( + await duplicateHostResponse.readAsString(), + 'Invalid Host header', + ); }); test('accepts loopback Host headers', () async { @@ -197,13 +250,22 @@ void main() { } }); - test('rejects non-loopback Origin headers with 403 Forbidden', () async { + test('rejects invalid Origin headers with 403 Forbidden', () async { const invalidOrigins = [ 'https://evil.com', 'http://evil.com:9100', 'http://localhost.evil.com', 'http://127.0.0.1.evil.com', 'http://localhost@evil.com', + 'file://localhost/foo', + 'custom://localhost', + 'ws://localhost:9104', + 'http://localhost:9104/', + 'http://localhost:9104/foo', + 'http://localhost:9104?a=1', + 'http://localhost:9104#frag', + 'http://', + 'http://:9104', 'null', '', ]; @@ -218,6 +280,17 @@ void main() { ); expect(await response.readAsString(), 'Invalid Origin header'); } + + final duplicateOriginResponse = await sendServerRequest( + headers: { + 'origin': ['http://localhost:54321', 'http://localhost:54321'], + }, + ); + expect(duplicateOriginResponse.statusCode, HttpStatus.forbidden); + expect( + await duplicateOriginResponse.readAsString(), + 'Invalid Origin header', + ); }); test('accepts loopback Origin headers', () async { @@ -241,16 +314,27 @@ void main() { }); test( - 'rejects Sec-Fetch-Site: cross-site without loopback Origin', + 'validates Sec-Fetch-Site based on presence of validated Origin', () async { - final response = await sendServerRequest( - headers: {'sec-fetch-site': 'cross-site'}, - ); - expect(response.statusCode, HttpStatus.forbidden); - expect( - await response.readAsString(), - 'Cross-site requests are not allowed', - ); + for (final disallowedWithoutOrigin in [ + 'cross-site', + 'same-site', + 'unexpected', + ]) { + final response = await sendServerRequest( + headers: {'sec-fetch-site': disallowedWithoutOrigin}, + ); + expect( + response.statusCode, + HttpStatus.forbidden, + reason: + 'Expected Sec-Fetch-Site "$disallowedWithoutOrigin" without Origin to be rejected', + ); + expect( + await response.readAsString(), + 'Cross-site requests are not allowed', + ); + } final evilOriginResponse = await sendServerRequest( headers: { @@ -271,27 +355,216 @@ void main() { HttpStatus.forbidden, ); - final crossPortLoopbackResponse = await sendServerRequest( - headers: { - HttpHeaders.hostHeader: '127.0.0.1:9104', - 'origin': 'http://localhost:54321', - 'sec-fetch-site': 'cross-site', - }, - ); - expect(crossPortLoopbackResponse.statusCode, HttpStatus.ok); + for (final allowedWithLoopbackOrigin in [ + 'same-origin', + 'same-site', + 'cross-site', + 'none', + ]) { + final response = await sendServerRequest( + headers: { + HttpHeaders.hostHeader: '127.0.0.1:9104', + 'origin': 'http://localhost:54321', + 'sec-fetch-site': allowedWithLoopbackOrigin, + }, + ); + expect( + response.statusCode, + HttpStatus.ok, + reason: + 'Expected Sec-Fetch-Site "$allowedWithLoopbackOrigin" with loopback Origin to be accepted', + ); + } - for (final allowed in ['same-origin', 'same-site', 'none']) { + for (final allowedWithoutOrigin in ['same-origin', 'none']) { final okResponse = await sendServerRequest( - headers: {'sec-fetch-site': allowed}, + headers: {'sec-fetch-site': allowedWithoutOrigin}, ); expect( okResponse.statusCode, HttpStatus.ok, - reason: 'Expected Sec-Fetch-Site "$allowed" to be accepted', + reason: + 'Expected Sec-Fetch-Site "$allowedWithoutOrigin" without Origin to be accepted', ); } }, ); + + test( + 'disableOriginCheck skips origin/host checks while keeping method and header checks', + () async { + final allowedPostResponse = await sendServerRequest( + uriHost: 'remote.example.com', + headers: { + HttpHeaders.hostHeader: 'remote.example.com:9104', + 'origin': 'https://evil.com', + 'sec-fetch-site': 'cross-site', + }, + disableOriginCheck: true, + ); + expect(allowedPostResponse.statusCode, HttpStatus.ok); + + final allowedNoOriginCrossSite = await sendServerRequest( + uriHost: 'remote.example.com', + headers: { + HttpHeaders.hostHeader: 'remote.example.com:9104', + 'sec-fetch-site': 'cross-site', + }, + disableOriginCheck: true, + ); + expect(allowedNoOriginCrossSite.statusCode, HttpStatus.ok); + + final allowedOptionsResponse = await sendServerRequest( + method: 'OPTIONS', + uriHost: 'remote.example.com', + headers: { + HttpHeaders.hostHeader: 'remote.example.com:9104', + 'origin': 'https://evil.com', + 'sec-fetch-site': 'cross-site', + }, + disableOriginCheck: true, + ); + expect(allowedOptionsResponse.statusCode, HttpStatus.noContent); + + final disallowedMethodResponse = await sendServerRequest( + method: 'GET', + uriHost: 'remote.example.com', + disableOriginCheck: true, + ); + expect( + disallowedMethodResponse.statusCode, + HttpStatus.methodNotAllowed, + ); + expect( + disallowedMethodResponse.headers[HttpHeaders.allowHeader], + 'POST, OPTIONS', + ); + + final missingHeaderResponse = await sendServerRequest( + uriHost: 'remote.example.com', + includeDevToolsRequestHeader: false, + disableOriginCheck: true, + ); + expect(missingHeaderResponse.statusCode, HttpStatus.forbidden); + expect( + await missingHeaderResponse.readAsString(), + 'Missing $devToolsRequestHeader header', + ); + }, + ); + + test( + 'supports non-loopback hosts and origins via allowedOrigins', + () async { + final allowedOrigins = [ + Uri.parse('http://192.168.1.50:9104/auth-token/'), + Uri.parse('https://dev.example.com'), + Uri.parse('http://[fe80::1]:9104'), + ]; + + final matchingPost = await sendServerRequest( + uriHost: '192.168.1.50', + headers: { + HttpHeaders.hostHeader: '192.168.1.50:9104', + 'origin': 'http://192.168.1.50:9104', + 'sec-fetch-site': 'same-origin', + }, + allowedOrigins: allowedOrigins, + ); + expect(matchingPost.statusCode, HttpStatus.ok); + + final matchingHttpsDefaultPort = await sendServerRequest( + uriHost: 'dev.example.com', + headers: { + HttpHeaders.hostHeader: 'dev.example.com', + 'origin': 'https://dev.example.com:443', + 'sec-fetch-site': 'same-site', + }, + allowedOrigins: allowedOrigins, + ); + expect(matchingHttpsDefaultPort.statusCode, HttpStatus.ok); + + final matchingIpv6 = await sendServerRequest( + uriHost: 'fe80::1', + headers: { + HttpHeaders.hostHeader: '[fe80::1]:9104', + 'origin': 'http://[fe80:0:0:0:0:0:0:1]:9104', + 'sec-fetch-site': 'same-origin', + }, + allowedOrigins: allowedOrigins, + ); + expect(matchingIpv6.statusCode, HttpStatus.ok); + + final matchingOptions = await sendServerRequest( + method: 'OPTIONS', + uriHost: '192.168.1.50', + headers: { + HttpHeaders.hostHeader: '192.168.1.50:9104', + 'origin': 'https://dev.example.com', + 'sec-fetch-site': 'cross-site', + }, + allowedOrigins: allowedOrigins, + ); + expect(matchingOptions.statusCode, HttpStatus.noContent); + + final loopbackStillAllowed = await sendServerRequest( + headers: { + HttpHeaders.hostHeader: '127.0.0.1:9104', + 'origin': 'http://localhost:54321', + 'sec-fetch-site': 'cross-site', + }, + allowedOrigins: allowedOrigins, + ); + expect(loopbackStillAllowed.statusCode, HttpStatus.ok); + + final wrongHost = await sendServerRequest( + uriHost: 'evil.com', + headers: {HttpHeaders.hostHeader: 'evil.com'}, + allowedOrigins: allowedOrigins, + ); + expect(wrongHost.statusCode, HttpStatus.forbidden); + expect(await wrongHost.readAsString(), 'Invalid Host header'); + + final wrongPortOrigin = await sendServerRequest( + uriHost: '192.168.1.50', + headers: { + HttpHeaders.hostHeader: '192.168.1.50:9104', + 'origin': 'http://192.168.1.50:5555', + }, + allowedOrigins: allowedOrigins, + ); + expect(wrongPortOrigin.statusCode, HttpStatus.forbidden); + expect(await wrongPortOrigin.readAsString(), 'Invalid Origin header'); + + final wrongSchemeOrigin = await sendServerRequest( + uriHost: 'dev.example.com', + headers: { + HttpHeaders.hostHeader: 'dev.example.com', + 'origin': 'http://dev.example.com', + }, + allowedOrigins: allowedOrigins, + ); + expect(wrongSchemeOrigin.statusCode, HttpStatus.forbidden); + expect( + await wrongSchemeOrigin.readAsString(), + 'Invalid Origin header', + ); + + final sameSiteWithoutOrigin = await sendServerRequest( + uriHost: 'dev.example.com', + headers: { + HttpHeaders.hostHeader: 'dev.example.com', + 'sec-fetch-site': 'same-site', + }, + allowedOrigins: allowedOrigins, + ); + expect(sameSiteWithoutOrigin.statusCode, HttpStatus.forbidden); + expect( + await sameSiteWithoutOrigin.readAsString(), + 'Cross-site requests are not allowed', + ); + }, + ); }); group(apiNotifyForVmServiceConnection, () { @@ -308,6 +581,7 @@ void main() { path: apiNotifyForVmServiceConnection, queryParameters: queryParameters, ), + headers: const {devToolsRequestHeader: '1'}, ); return server.ServerApi.handle( request, diff --git a/pubspec.lock b/pubspec.lock index 59f29e3493c..9429b8a44fc 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -899,26 +899,26 @@ packages: dependency: transitive description: name: test - sha256: "44b2fccf4407bcdb5e105c064c5d6b5e2a640c87761f8b03cdce8cf532d98cfa" + sha256: de5d145b0afff7921e5e788a880f52d7e5f3ae24068a202f6fd3b58e4ba26323 url: "https://pub.dev" source: hosted - version: "1.33.0" + version: "1.32.0" test_api: dependency: transitive description: name: test_api - sha256: "47481c58bb92d0b1d218f0510ff5647e768f8e1256d07077a151fb520923b51d" + sha256: "0a10344e901e5b2e63819567951cb6a06673ed6b84f40462188ff5a0c41f371f" url: "https://pub.dev" source: hosted - version: "0.7.15" + version: "0.7.14" test_core: dependency: transitive description: name: test_core - sha256: "9dec4c178bd61493e1cd2060ffe701fcb61c43da2493e99590fb1b0cc82eb4af" + sha256: "80f3fb49087454e07e7e07c67578cfdd156c8c3a5227d8b3f47c7b2d019c2e93" url: "https://pub.dev" source: hosted - version: "0.6.21" + version: "0.6.20" typed_data: dependency: transitive description: