This change hardens DTD FileSystemService workspace-root enforcement against symlink escapes.
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 <yjessy@google.com> Auto-Submit: 진호 <orangemush777@gmail.com> Reviewed-by: Ben Konyi <bkonyi@google.com> Commit-Queue: Ben Konyi <bkonyi@google.com>
This commit is contained in:
@@ -66,13 +66,16 @@ class FileSystemService extends InternalService {
|
||||
_ideWorkspaceRoots.clear();
|
||||
}
|
||||
|
||||
void _ensureIDEWorkspaceRootsContainUri(Uri uri) {
|
||||
Future<void> _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<String> _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<String> _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<String, Object?> _setIDEWorkspaceRoots(Parameters parameters) {
|
||||
final incomingSecret = parameters[DtdParameters.secret].asString;
|
||||
|
||||
@@ -157,7 +200,7 @@ class FileSystemService extends InternalService {
|
||||
|
||||
Future<Map<String, Object?>> _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(
|
||||
|
||||
@@ -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<RpcException>().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<RpcException>().having(
|
||||
(e) => e.code,
|
||||
'code',
|
||||
RpcErrorCodes.kPermissionDenied,
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
expect(
|
||||
() => client.sendRequest(
|
||||
'${FileSystemServiceConstants.serviceName}.'
|
||||
'${FileSystemServiceConstants.listDirectoryContents}',
|
||||
{'uri': Directory(escapeLink.path).uri.toString()},
|
||||
),
|
||||
throwsA(
|
||||
isA<RpcException>().having(
|
||||
(e) => e.code,
|
||||
'code',
|
||||
RpcErrorCodes.kPermissionDenied,
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
expect(secretFile.readAsStringSync(), 'TOP-SECRET');
|
||||
expect(File('${outside.path}/created.txt').existsSync(), isFalse);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('dtd arguments', () {
|
||||
|
||||
Reference in New Issue
Block a user