diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md index 0f1f770d1a9..f6fa2bc2db5 100644 --- a/packages/devtools_shared/CHANGELOG.md +++ b/packages/devtools_shared/CHANGELOG.md @@ -3,6 +3,11 @@ Copyright 2025 The Flutter Authors Use of this source code is governed by a BSD-style license that can be found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. --> +# 13.0.2 +* Validate the `devtoolsOptionsUri` query parameter in the extension enabled + state handler so it must be a `file:` URI named `devtools_options.yaml`, + preventing arbitrary file writes by the DevTools server process. + # 13.0.1 * Handle null values for `FlutterStore.flutterClientId`. diff --git a/packages/devtools_shared/lib/src/server/handlers/_devtools_extensions.dart b/packages/devtools_shared/lib/src/server/handlers/_devtools_extensions.dart index f7c63671630..e87e2704d23 100644 --- a/packages/devtools_shared/lib/src/server/handlers/_devtools_extensions.dart +++ b/packages/devtools_shared/lib/src/server/handlers/_devtools_extensions.dart @@ -80,6 +80,27 @@ extension _ExtensionsApiHandler on Never { final devtoolsOptionsFileUriString = queryParams[ExtensionsApi.devtoolsOptionsUriPropertyName]!; final devtoolsOptionsFileUri = Uri.parse(devtoolsOptionsFileUriString); + + // Validate that the URI is a local file URI whose file name is exactly + // 'devtools_options.yaml'. Accepting arbitrary URIs from the query string + // would allow an untrusted caller to create or overwrite any file writable + // by the DevTools server process. Resolving the name through + // `Uri.toFilePath()` + `p.basename` handles both '/' and '\' path + // separators, so the check holds for Windows file URIs as well. Requiring + // an empty host rejects UNC paths (e.g. `file://server/share/...`) and + // keeps `toFilePath()` from throwing on a non-local authority. + final isFileUri = devtoolsOptionsFileUri.scheme == 'file' && + devtoolsOptionsFileUri.host.isEmpty; + final fileName = isFileUri + ? p.basename(devtoolsOptionsFileUri.toFilePath()) + : ''; + if (!isFileUri || fileName != 'devtools_options.yaml') { + return api.badRequest( + 'Invalid devtoolsOptionsUri: must be a file: URI named ' + "'devtools_options.yaml'.", + ); + } + final extensionName = queryParams[ExtensionsApi.extensionNamePropertyName]!; final activate = queryParams[ExtensionsApi.enabledStatePropertyName]; diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index f9b50724845..cc41fb0d9e7 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -11,6 +11,7 @@ import 'dart:io'; import 'package:collection/collection.dart'; import 'package:dtd/dtd.dart'; import 'package:meta/meta.dart'; +import 'package:path/path.dart' as p; import 'package:shelf/shelf.dart' as shelf; import 'package:vm_service/vm_service.dart'; diff --git a/packages/devtools_shared/pubspec.yaml b/packages/devtools_shared/pubspec.yaml index dce148c2109..585ad854798 100644 --- a/packages/devtools_shared/pubspec.yaml +++ b/packages/devtools_shared/pubspec.yaml @@ -4,7 +4,7 @@ name: devtools_shared description: Package of shared Dart structures between devtools_app, dds, and other tools. -version: 13.0.1 +version: 13.0.2 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_shared 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 f075b4717e1..963744fa214 100644 --- a/packages/devtools_shared/test/server/devtools_extensions_api_test.dart +++ b/packages/devtools_shared/test/server/devtools_extensions_api_test.dart @@ -49,10 +49,9 @@ void main() { includeDependenciesWithExtensions: includeDependenciesWithExtensions, includeBadExtension: includeBadExtension, ); - await testDtdConnection!.setIDEWorkspaceRoots( - dtd!.info!.secret!, - [extensionTestManager.packagesRootUri], - ); + await testDtdConnection!.setIDEWorkspaceRoots(dtd!.info!.secret!, [ + extensionTestManager.packagesRootUri, + ]); } Future serveExtensions( @@ -66,8 +65,9 @@ void main() { host: 'localhost', path: ExtensionsApi.apiServeAvailableExtensions, queryParameters: { - ExtensionsApi.packageRootUriPropertyName: - includeRuntimeRoot ? extensionTestManager.runtimeAppRoot : null, + ExtensionsApi.packageRootUriPropertyName: includeRuntimeRoot + ? extensionTestManager.runtimeAppRoot + : null, }, ), ); @@ -128,9 +128,7 @@ void main() { test( 'fails when an exception is thrown and there are no valid extensions', () async { - await initializeTestDirectory( - includeDependenciesWithExtensions: false, - ); + await initializeTestDirectory(includeDependenciesWithExtensions: false); extensionsManager = _TestExtensionsManager(); final response = await serveExtensions(extensionsManager); expect(response.statusCode, HttpStatus.internalServerError); @@ -159,6 +157,7 @@ void main() { Future sendEnabledStateRequest({ required String extensionName, bool? enable, + String? optionsUriOverride, }) async { final request = Request( 'post', @@ -167,7 +166,8 @@ void main() { host: 'localhost', path: ExtensionsApi.apiExtensionEnabledState, queryParameters: { - ExtensionsApi.devtoolsOptionsUriPropertyName: optionsFileUriString, + ExtensionsApi.devtoolsOptionsUriPropertyName: + optionsUriOverride ?? optionsFileUriString, ExtensionsApi.extensionNamePropertyName: extensionName, if (enable != null) ExtensionsApi.enabledStatePropertyName: enable.toString(), @@ -181,6 +181,37 @@ void main() { ); } + test('rejects a devtoolsOptionsUri that is not a devtools_options.yaml ' + 'file', () async { + await serveExtensions(extensionsManager); + const invalidUris = [ + // Wrong file name. + 'file:///tmp/evil.txt', + 'file:///tmp/devtools_options.yaml.bak', + // Non-file scheme. + 'https://evil.example.com/devtools_options.yaml', + // UNC path / non-empty host. + 'file://remotehost/share/devtools_options.yaml', + ]; + for (final invalidUri in invalidUris) { + final response = await sendEnabledStateRequest( + extensionName: 'drift', + optionsUriOverride: invalidUri, + ); + expect( + response.statusCode, + HttpStatus.badRequest, + reason: 'expected $invalidUri to be rejected', + ); + } + }); + + test('accepts a valid devtools_options.yaml file: URI', () async { + await serveExtensions(extensionsManager); + final response = await sendEnabledStateRequest(extensionName: 'drift'); + expect(response.statusCode, HttpStatus.ok); + }); + test('options file does not exist until first acesss', () async { await serveExtensions(extensionsManager); expect(optionsFile.existsSync(), isFalse); @@ -202,16 +233,16 @@ void main() { ExtensionEnabledState.none.name, ); -// TODO(kenz): why is existsSync() returning false when I can verify the file -// contents on the file system at [optionsFileUriString]? -// expect(optionsFile.existsSync(), isTrue); -// expect( -// optionsFile.readAsStringSync(), -// ''' -// description: This file stores settings for Dart & Flutter DevTools. -// documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states -// extensions:''', -// ); + // TODO(kenz): why is existsSync() returning false when I can verify the file + // contents on the file system at [optionsFileUriString]? + // expect(optionsFile.existsSync(), isTrue); + // expect( + // optionsFile.readAsStringSync(), + // ''' + // description: This file stores settings for Dart & Flutter DevTools. + // documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states + // extensions:''', + // ); response = await sendEnabledStateRequest( extensionName: 'drift', @@ -233,16 +264,16 @@ void main() { ExtensionEnabledState.disabled.name, ); -// expect(optionsFile.existsSync(), isTrue); -// expect( -// optionsFile.readAsStringSync(), -// ''' -// description: This file stores settings for Dart & Flutter DevTools. -// documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states -// extensions: -// - drift: true -// - provider: false''', -// ); + // expect(optionsFile.existsSync(), isTrue); + // expect( + // optionsFile.readAsStringSync(), + // ''' + // description: This file stores settings for Dart & Flutter DevTools. + // documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states + // extensions: + // - drift: true + // - provider: false''', + // ); }); }); }