From 046fc17e533c2a126bc0df20a49ccff220c668a9 Mon Sep 17 00:00:00 2001 From: Jinho Seo Date: Wed, 8 Apr 2026 10:13:43 -0700 Subject: [PATCH] This change hardens DTD FileSystemService workspace-root enforcement against symlink escapes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, FileSystem.readFileAsString, writeFileAsString, and listDirectoryContents only validated the requested path textually against the configured IDE workspace roots. A symlink inside the workspace could therefore resolve to a location outside the workspace and still be accessed. This patch resolves workspace roots and requested filesystem targets before performing the authorization check, and resolves the nearest existing ancestor for write targets so new files inside a workspace continue to work. It also adds a source-backed regression test in pkg/dtd_impl/test/dtd_test.dart that verifies read, write, and directory listing requests through a symlink escaping the workspace all fail with permission denied. R=bquinlan@google.com Tested: - HOME=/tmp XDG_CONFIG_HOME=/tmp DART_SUPPRESS_ANALYTICS=1 /tmp/dart-sdk-3.12.0-221.0.dev/dart-sdk/bin/dart test pkg/dtd_impl/test/dtd_test.dart Change-Id: I7abf00f6220bff42b352e2942f396167af53adb8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493420 Reviewed-by: Jessy Yameogo Auto-Submit: 진호 Reviewed-by: Ben Konyi Commit-Queue: Ben Konyi --- .../lib/src/service/file_system_service.dart | 57 +++++++++++-- pkg/dtd_impl/test/dtd_test.dart | 80 +++++++++++++++++++ 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/pkg/dtd_impl/lib/src/service/file_system_service.dart b/pkg/dtd_impl/lib/src/service/file_system_service.dart index 5cc4a696c72..55f37e9ba54 100644 --- a/pkg/dtd_impl/lib/src/service/file_system_service.dart +++ b/pkg/dtd_impl/lib/src/service/file_system_service.dart @@ -66,13 +66,16 @@ class FileSystemService extends InternalService { _ideWorkspaceRoots.clear(); } - void _ensureIDEWorkspaceRootsContainUri(Uri uri) { + Future _ensureIDEWorkspaceRootsContainUri(Uri uri) async { // If in unrestricted mode, no need to do these checks. if (unrestrictedMode) return; - final requestedPath = uri.toFilePath(); - if (_ideWorkspaceRoots.any((root) { - final rootPath = root.toFilePath(); + final requestedPath = await _resolveNearestExistingPath(uri.toFilePath()); + final resolvedWorkspaceRoots = await Future.wait( + _ideWorkspaceRoots.map(_resolveWorkspaceRootPath), + ); + + if (resolvedWorkspaceRoots.any((rootPath) { return path.isWithin(rootPath, requestedPath) || path.equals(rootPath, requestedPath); })) { @@ -82,6 +85,46 @@ class FileSystemService extends InternalService { throw RpcErrorCodes.buildRpcException(RpcErrorCodes.kPermissionDenied); } + Future _resolveWorkspaceRootPath(Uri root) async { + final rootPath = root.toFilePath(); + final type = await FileSystemEntity.type(rootPath, followLinks: true); + if (type == FileSystemEntityType.file) { + return File(rootPath).resolveSymbolicLinks(); + } + if (type == FileSystemEntityType.directory) { + return Directory(rootPath).resolveSymbolicLinks(); + } + if (type == FileSystemEntityType.link) { + return Link(rootPath).resolveSymbolicLinks(); + } + return path.normalize(rootPath); + } + + Future _resolveNearestExistingPath(String requestedPath) async { + var currentPath = requestedPath; + while (true) { + final type = await FileSystemEntity.type(currentPath, followLinks: true); + if (type != FileSystemEntityType.notFound) { + if (type == FileSystemEntityType.file) { + return File(currentPath).resolveSymbolicLinks(); + } + if (type == FileSystemEntityType.directory) { + return Directory(currentPath).resolveSymbolicLinks(); + } + if (type == FileSystemEntityType.link) { + return Link(currentPath).resolveSymbolicLinks(); + } + throw StateError('Unexpected file system entity type: $type'); + } + + final parentPath = path.dirname(currentPath); + if (path.equals(parentPath, currentPath)) { + return path.normalize(currentPath); + } + currentPath = parentPath; + } + } + Map _setIDEWorkspaceRoots(Parameters parameters) { final incomingSecret = parameters[DtdParameters.secret].asString; @@ -157,7 +200,7 @@ class FileSystemService extends InternalService { Future> _readFileAsString(Parameters parameters) async { final uri = _extractUri(parameters); - _ensureIDEWorkspaceRootsContainUri(uri); + await _ensureIDEWorkspaceRootsContainUri(uri); final file = File.fromUri(uri); if (!(await file.exists())) { @@ -187,7 +230,7 @@ class FileSystemService extends InternalService { parameters[DtdParameters.encoding].asString, )!; - _ensureIDEWorkspaceRootsContainUri(uri); + await _ensureIDEWorkspaceRootsContainUri(uri); final file = File.fromUri(uri); if (!(await file.exists())) { await file.create(recursive: true); @@ -202,7 +245,7 @@ class FileSystemService extends InternalService { Parameters parameters, ) async { final uri = _extractUri(parameters); - _ensureIDEWorkspaceRootsContainUri(uri); + await _ensureIDEWorkspaceRootsContainUri(uri); final dir = Directory.fromUri(uri); if (!(await dir.exists())) { throw RpcErrorCodes.buildRpcException( diff --git a/pkg/dtd_impl/test/dtd_test.dart b/pkg/dtd_impl/test/dtd_test.dart index 089d532cf34..fda8f2f24f5 100644 --- a/pkg/dtd_impl/test/dtd_test.dart +++ b/pkg/dtd_impl/test/dtd_test.dart @@ -527,6 +527,86 @@ void main() { }); }); }); + + group('file system service', () { + test('denies symlink escapes from IDE workspace roots', () async { + final tempRoot = await Directory.systemTemp.createTemp('dtd_test.'); + final workspace = Directory('${tempRoot.path}/workspace')..createSync(); + final outside = Directory('${tempRoot.path}/outside')..createSync(); + final secretFile = File('${outside.path}/secret.txt') + ..createSync() + ..writeAsStringSync('TOP-SECRET'); + final escapeLink = Link('${workspace.path}/escape-link') + ..createSync(outside.path, recursive: true); + + addTearDown(() async { + if (tempRoot.existsSync()) { + await tempRoot.delete(recursive: true); + } + }); + + await client.sendRequest( + '${FileSystemServiceConstants.serviceName}.' + '${FileSystemServiceConstants.setIDEWorkspaceRoots}', + { + 'secret': dtd!.secret, + 'roots': [workspace.uri.toString()], + }, + ); + + expect( + () => client.sendRequest( + '${FileSystemServiceConstants.serviceName}.' + '${FileSystemServiceConstants.readFileAsString}', + {'uri': File('${escapeLink.path}/secret.txt').uri.toString()}, + ), + throwsA( + isA().having( + (e) => e.code, + 'code', + RpcErrorCodes.kPermissionDenied, + ), + ), + ); + + expect( + () => client.sendRequest( + '${FileSystemServiceConstants.serviceName}.' + '${FileSystemServiceConstants.writeFileAsString}', + { + 'uri': File('${escapeLink.path}/created.txt').uri.toString(), + 'contents': 'WRITE-THROUGH-LINK', + 'encoding': 'utf-8', + }, + ), + throwsA( + isA().having( + (e) => e.code, + 'code', + RpcErrorCodes.kPermissionDenied, + ), + ), + ); + + expect( + () => client.sendRequest( + '${FileSystemServiceConstants.serviceName}.' + '${FileSystemServiceConstants.listDirectoryContents}', + {'uri': Directory(escapeLink.path).uri.toString()}, + ), + throwsA( + isA().having( + (e) => e.code, + 'code', + RpcErrorCodes.kPermissionDenied, + ), + ), + ); + + expect(secretFile.readAsStringSync(), 'TOP-SECRET'); + expect(File('${outside.path}/created.txt').existsSync(), isFalse); + }); + }); }); group('dtd arguments', () {