Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions lib/core/extensions/local_extension_installer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:querya_desktop/core/extensions/local_extension_registry.dart';
import 'package:querya_desktop/core/extensions/models/extension_manifest.dart';
import 'package:querya_desktop/core/extensions/sandbox/sandbox_policy.dart';
import 'package:querya_desktop/core/market/marketplace_repository.dart';
import 'package:querya_desktop/core/security/archive_path_guard.dart';

/// Installs an extension package from a local `.zip` / `.qext` archive (issue #316).
///
Expand Down Expand Up @@ -216,17 +217,15 @@ class LocalExtensionInstaller {
}
if (filename.isEmpty || filename == '/') continue;

if (filename.contains('..') ||
filename.startsWith('/') ||
filename.startsWith('\\')) {
if (!isArchiveEntryNameSafe(filename)) {
throw MarketplaceException(
'Security violation: Path traversal detected in archive entry '
'"${file.name}"',
);
}

final targetPath = p.normalize(p.join(destPath, filename));
if (!targetPath.startsWith(destPath)) {
if (!isArchiveExtractPathWithinRoot(destPath, targetPath)) {
throw MarketplaceException(
'Security violation: Extraction path out of bounds "${file.name}"',
);
Expand Down
5 changes: 3 additions & 2 deletions lib/core/market/http_marketplace_repository.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:querya_desktop/core/extensions/sandbox/sandbox_policy.dart';
import 'package:querya_desktop/core/extensions/local_extension_registry.dart';
import 'package:querya_desktop/core/extensions/models/extension_manifest.dart';
import 'package:querya_desktop/core/extensions/models/extension_type.dart';
import 'package:querya_desktop/core/security/archive_path_guard.dart';
import 'marketplace_repository.dart';

/// HTTP implementation of [MarketplaceRepository] connecting to MarketApi backend.
Expand Down Expand Up @@ -152,12 +153,12 @@ class HttpMarketplaceRepository implements MarketplaceRepository {
for (final file in archive) {
final filename = file.name;
// Check for Path Traversal attempts
if (filename.contains('..') || filename.startsWith('/') || filename.startsWith('\\')) {
if (!isArchiveEntryNameSafe(filename)) {
throw MarketplaceException('Security violation: Path traversal detected in archive entry "$filename"');
}

final targetPath = p.normalize(p.join(extDirPath, filename));
if (!targetPath.startsWith(extDirPath)) {
if (!isArchiveExtractPathWithinRoot(extDirPath, targetPath)) {
throw MarketplaceException('Security violation: Extraction path out of bounds "$filename"');
}

Expand Down
18 changes: 18 additions & 0 deletions lib/core/security/archive_path_guard.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import 'package:path/path.dart' as p;

/// Returns true when [targetPath] equals [rootPath] or lies inside it.
///
/// Prefer over [String.startsWith] so sibling prefixes (e.g. `/tmp/abc` vs
/// `/tmp/abcd`) cannot bypass extraction bounds.
bool isArchiveExtractPathWithinRoot(String rootPath, String targetPath) {
final root = p.normalize(rootPath);
final target = p.normalize(targetPath);
return p.equals(root, target) || p.isWithin(root, target);
}

/// Rejects archive entry names that attempt absolute paths or traversal.
bool isArchiveEntryNameSafe(String entryName) {
if (entryName.contains('..')) return false;
if (entryName.startsWith('/') || entryName.startsWith('\\')) return false;
return true;
}
5 changes: 3 additions & 2 deletions lib/core/updater/installers/update_install_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:io';
import 'package:archive/archive.dart';
import 'package:path/path.dart' as p;

import '../../security/archive_path_guard.dart';
import '../app_updater_service.dart';

/// Safely extracts a zip archive into [destinationDir].
Expand All @@ -21,14 +22,14 @@ Future<void> extractZipSecurely({

for (final entry in archive) {
final name = entry.name;
if (name.contains('..') || name.startsWith('/') || name.startsWith('\\')) {
if (!isArchiveEntryNameSafe(name)) {
throw AppUpdaterException(
'Security violation: path traversal in archive entry "$name"',
);
}

final targetPath = p.normalize(p.join(root, name));
if (!targetPath.startsWith(root)) {
if (!isArchiveExtractPathWithinRoot(root, targetPath)) {
throw AppUpdaterException(
'Security violation: extraction path out of bounds "$name"',
);
Expand Down
47 changes: 47 additions & 0 deletions test/core/security/archive_path_guard_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:querya_desktop/core/security/archive_path_guard.dart';

void main() {
group('isArchiveExtractPathWithinRoot', () {
test('allows paths inside root', () {
expect(
isArchiveExtractPathWithinRoot('/tmp/ext', '/tmp/ext/file.txt'),
isTrue,
);
});

test('allows root path itself', () {
expect(
isArchiveExtractPathWithinRoot('/tmp/ext', '/tmp/ext'),
isTrue,
);
});

test('rejects sibling prefix paths (startsWith false positive)', () {
expect(
isArchiveExtractPathWithinRoot('/tmp/abc', '/tmp/abcd/evil.txt'),
isFalse,
);
});

test('rejects paths outside root', () {
expect(
isArchiveExtractPathWithinRoot('/tmp/ext', '/tmp/other/file.txt'),
isFalse,
);
});
});

group('isArchiveEntryNameSafe', () {
test('rejects traversal and absolute names', () {
expect(isArchiveEntryNameSafe('../evil.txt'), isFalse);
expect(isArchiveEntryNameSafe('/etc/passwd'), isFalse);
expect(isArchiveEntryNameSafe(r'\windows\system32'), isFalse);
});

test('allows relative safe names', () {
expect(isArchiveEntryNameSafe('manifest.json'), isTrue);
expect(isArchiveEntryNameSafe('bin/driver'), isTrue);
});
});
}
Loading