Merge pull request #404 from QueryaHub/issue/401-archive-is-within

fix(security): use p.isWithin for archive extraction bounds

Author: ZhuchkaTriplesix

lib/core/extensions/local_extension_installer.dart

@@ -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).
 ///
@@ -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}"',
         );

lib/core/market/http_marketplace_repository.dart

@@ -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.
@@ -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"');
         }
 

lib/core/security/archive_path_guard.dart

@@ -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;
+}

lib/core/updater/installers/update_install_utils.dart

@@ -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].
@@ -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"',
       );

test/core/security/archive_path_guard_test.dart

@@ -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);
+    });
+  });
+}