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/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 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',