From 1d96aa32afeccc03bd74fbb741bb84376f543275 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Mon, 27 Jul 2026 06:44:55 +0300 Subject: [PATCH 1/2] fix(security): add SafeZipExtractor limits for archive installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #398 — shared zip bomb bounds for marketplace, sideload, and updater paths; document default limits in docs/security.md. --- docs/security.md | 14 ++ .../extensions/local_extension_installer.dart | 15 +- .../market/http_marketplace_repository.dart | 17 ++- lib/core/security/safe_zip_extractor.dart | 134 +++++++++++++++++ .../installers/update_install_utils.dart | 9 +- .../security/safe_zip_extractor_test.dart | 136 ++++++++++++++++++ 6 files changed, 318 insertions(+), 7 deletions(-) create mode 100644 lib/core/security/safe_zip_extractor.dart create mode 100644 test/core/security/safe_zip_extractor_test.dart diff --git a/docs/security.md b/docs/security.md index 9b9d44a7..545fbabf 100644 --- a/docs/security.md +++ b/docs/security.md @@ -21,3 +21,17 @@ On upgrade from older databases, existing plaintext secrets in SQLite are **migr ## Tests Automated tests use an **in-memory** secrets backend (see `test/flutter_test_config.dart`) so CI does not require a desktop keyring. + +## Archive install limits (extensions and updates) + +Marketplace downloads, local extension sideload (`.zip` / `.qext`), and in-app updater extraction use `SafeZipExtractor` (`lib/core/security/safe_zip_extractor.dart`) with shared default limits: + +| Limit | Default | +|-------|---------| +| Max compressed archive size | 100 MiB | +| Max total uncompressed size | 500 MiB | +| Max entries | 10 000 | +| Max single entry uncompressed size | 100 MiB | +| Max compression ratio (uncompressed ÷ compressed) | 100:1 | + +Archives exceeding these bounds fail closed before files are written to disk. Path traversal checks remain in `archive_path_guard.dart`. diff --git a/lib/core/extensions/local_extension_installer.dart b/lib/core/extensions/local_extension_installer.dart index f413494c..3296763f 100644 --- a/lib/core/extensions/local_extension_installer.dart +++ b/lib/core/extensions/local_extension_installer.dart @@ -11,6 +11,7 @@ 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'; +import 'package:querya_desktop/core/security/safe_zip_extractor.dart'; /// Installs an extension package from a local `.zip` / `.qext` archive (issue #316). /// @@ -43,7 +44,12 @@ class LocalExtensionInstaller { } onProgress?.call(0.1); - final bytes = await archiveFile.readAsBytes(); + late final List bytes; + try { + bytes = await SafeZipExtractor.readBoundedBytes(archiveFile); + } on SafeZipException catch (error) { + throw MarketplaceException(error.message); + } if (expectedSha256 != null && expectedSha256.trim().isNotEmpty) { final actual = sha256.convert(bytes).toString().toLowerCase(); @@ -57,7 +63,12 @@ class LocalExtensionInstaller { } onProgress?.call(0.25); - final archive = ZipDecoder().decodeBytes(bytes); + late final Archive archive; + try { + archive = SafeZipExtractor.decodeBytes(bytes); + } on SafeZipException catch (error) { + throw MarketplaceException(error.message); + } if (archive.isEmpty) { throw MarketplaceException('Extension archive is empty.'); } diff --git a/lib/core/market/http_marketplace_repository.dart b/lib/core/market/http_marketplace_repository.dart index ee4c29d7..11578fb7 100644 --- a/lib/core/market/http_marketplace_repository.dart +++ b/lib/core/market/http_marketplace_repository.dart @@ -13,6 +13,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/models/extension_type.dart'; import 'package:querya_desktop/core/security/archive_path_guard.dart'; +import 'package:querya_desktop/core/security/safe_zip_extractor.dart'; import 'marketplace_download_policy.dart'; import 'marketplace_repository.dart'; @@ -170,7 +171,12 @@ class HttpMarketplaceRepository implements MarketplaceRepository { ); } - final bytes = await archiveFile.readAsBytes(); + late final List bytes; + try { + bytes = await SafeZipExtractor.readBoundedBytes(archiveFile); + } on SafeZipException catch (error) { + throw MarketplaceException(error.message); + } final actualSha256 = sha256.convert(bytes).toString().toLowerCase(); if (actualSha256 != expectedSha256) { throw MarketplaceException( @@ -181,8 +187,13 @@ class HttpMarketplaceRepository implements MarketplaceRepository { onProgress?.call(0.85); - // Step 3: Safe Archive Extraction (Preventing Path Traversal / Zip Bomb - Issue #242) - final archive = ZipDecoder().decodeBytes(bytes); + // Step 3: Safe Archive Extraction (path traversal + zip bomb limits) + final Archive archive; + try { + archive = SafeZipExtractor.decodeBytes(bytes); + } on SafeZipException catch (error) { + throw MarketplaceException(error.message); + } final dir = await ExtensionPaths.extensionsDirectory(); final extDir = Directory(p.join(dir.path, manifest.id)); diff --git a/lib/core/security/safe_zip_extractor.dart b/lib/core/security/safe_zip_extractor.dart new file mode 100644 index 00000000..0ff52fad --- /dev/null +++ b/lib/core/security/safe_zip_extractor.dart @@ -0,0 +1,134 @@ +import 'dart:io'; + +import 'package:archive/archive.dart'; + +/// Bounds for zip decode/extract to mitigate zip bombs and memory exhaustion. +class ZipDecodeLimits { + const ZipDecodeLimits({ + required this.maxCompressedBytes, + required this.maxTotalUncompressedBytes, + required this.maxEntryCount, + required this.maxEntryUncompressedBytes, + required this.maxCompressionRatio, + }); + + final int maxCompressedBytes; + final int maxTotalUncompressedBytes; + final int maxEntryCount; + final int maxEntryUncompressedBytes; + final double maxCompressionRatio; + + /// Default limits for marketplace, sideload, and updater archives. + static const ZipDecodeLimits standard = ZipDecodeLimits( + maxCompressedBytes: 100 * 1024 * 1024, + maxTotalUncompressedBytes: 500 * 1024 * 1024, + maxEntryCount: 10000, + maxEntryUncompressedBytes: 100 * 1024 * 1024, + maxCompressionRatio: 100, + ); +} + +/// Thrown when an archive exceeds [ZipDecodeLimits]. +class SafeZipException implements Exception { + SafeZipException(this.message); + + final String message; + + @override + String toString() => 'SafeZipException: $message'; +} + +/// Bounded zip decode used by marketplace, sideload, and updater paths. +abstract final class SafeZipExtractor { + static Future> readBoundedBytes( + File file, { + ZipDecodeLimits limits = ZipDecodeLimits.standard, + }) async { + final length = await file.length(); + if (length > limits.maxCompressedBytes) { + throw SafeZipException( + 'Archive exceeds maximum compressed size ' + '(${limits.maxCompressedBytes} bytes).', + ); + } + return file.readAsBytes(); + } + + static Archive decodeBytes( + List bytes, { + ZipDecodeLimits limits = ZipDecodeLimits.standard, + }) { + if (bytes.length > limits.maxCompressedBytes) { + throw SafeZipException( + 'Archive exceeds maximum compressed size ' + '(${limits.maxCompressedBytes} bytes).', + ); + } + + final Archive archive; + try { + archive = ZipDecoder().decodeBytes(bytes); + } on Object catch (error) { + throw SafeZipException('Failed to decode zip archive: $error'); + } + + _validateArchive( + archive, + compressedBytes: bytes.length, + limits: limits, + ); + return archive; + } + + static Future readAndDecodeFile( + File file, { + ZipDecodeLimits limits = ZipDecodeLimits.standard, + }) async { + final bytes = await readBoundedBytes(file, limits: limits); + return decodeBytes(bytes, limits: limits); + } + + static void _validateArchive( + Archive archive, { + required int compressedBytes, + required ZipDecodeLimits limits, + }) { + if (archive.length > limits.maxEntryCount) { + throw SafeZipException( + 'Archive contains too many entries (${archive.length}; ' + 'max ${limits.maxEntryCount}).', + ); + } + + var totalUncompressed = 0; + for (final entry in archive) { + if (!entry.isFile) continue; + + final size = entry.size; + if (size > limits.maxEntryUncompressedBytes) { + throw SafeZipException( + 'Archive entry "${entry.name}" exceeds maximum uncompressed size ' + '($size bytes; max ${limits.maxEntryUncompressedBytes}).', + ); + } + + totalUncompressed += size; + if (totalUncompressed > limits.maxTotalUncompressedBytes) { + throw SafeZipException( + 'Archive exceeds maximum total uncompressed size ' + '(max ${limits.maxTotalUncompressedBytes} bytes).', + ); + } + } + + if (compressedBytes > 0 && totalUncompressed > 0) { + final ratio = totalUncompressed / compressedBytes; + if (ratio > limits.maxCompressionRatio) { + throw SafeZipException( + 'Archive compression ratio is too high ' + '(${ratio.toStringAsFixed(1)}:1; max ${limits.maxCompressionRatio}:1).', + ); + } + } + } +} diff --git a/lib/core/updater/installers/update_install_utils.dart b/lib/core/updater/installers/update_install_utils.dart index a5a769f9..6755191e 100644 --- a/lib/core/updater/installers/update_install_utils.dart +++ b/lib/core/updater/installers/update_install_utils.dart @@ -4,6 +4,7 @@ import 'package:archive/archive.dart'; import 'package:path/path.dart' as p; import '../../security/archive_path_guard.dart'; +import '../../security/safe_zip_extractor.dart'; import '../app_updater_service.dart'; /// Safely extracts a zip archive into [destinationDir]. @@ -16,8 +17,12 @@ Future extractZipSecurely({ } await destinationDir.create(recursive: true); - final bytes = await zipFile.readAsBytes(); - final archive = ZipDecoder().decodeBytes(bytes); + final Archive archive; + try { + archive = await SafeZipExtractor.readAndDecodeFile(zipFile); + } on SafeZipException catch (error) { + throw AppUpdaterException(error.message); + } final root = p.normalize(destinationDir.path); for (final entry in archive) { diff --git a/test/core/security/safe_zip_extractor_test.dart b/test/core/security/safe_zip_extractor_test.dart new file mode 100644 index 00000000..56fab3e7 --- /dev/null +++ b/test/core/security/safe_zip_extractor_test.dart @@ -0,0 +1,136 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:archive/archive.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; +import 'package:querya_desktop/core/security/safe_zip_extractor.dart'; + +const _tightLimits = ZipDecodeLimits( + maxCompressedBytes: 4096, + maxTotalUncompressedBytes: 8192, + maxEntryCount: 5, + maxEntryUncompressedBytes: 4096, + maxCompressionRatio: 10, +); + +Archive _singleFileArchive(String name, List content) { + return Archive()..addFile(ArchiveFile(name, content.length, content)); +} + +Future _writeZip(Directory dir, Archive archive, String name) async { + final bytes = ZipEncoder().encode(archive)!; + final file = File(p.join(dir.path, name)); + await file.writeAsBytes(bytes); + return file; +} + +void main() { + group('SafeZipExtractor', () { + late Directory tempDir; + + setUp(() async { + tempDir = await Directory.systemTemp.createTemp('querya_safe_zip_'); + }); + + tearDown(() async { + if (await tempDir.exists()) { + await tempDir.delete(recursive: true); + } + }); + + test('decodes a small valid archive', () { + final archive = _singleFileArchive('hello.txt', utf8.encode('hello')); + final zipBytes = ZipEncoder().encode(archive)!; + + final decoded = SafeZipExtractor.decodeBytes(zipBytes, limits: _tightLimits); + expect(decoded.length, 1); + expect(decoded.first.name, 'hello.txt'); + }); + + test('rejects archives exceeding max compressed bytes', () async { + final file = File(p.join(tempDir.path, 'oversize.zip')); + await file.writeAsBytes(List.filled(5000, 1)); + + expect( + () => SafeZipExtractor.readBoundedBytes(file, limits: _tightLimits), + throwsA(isA().having( + (e) => e.message, + 'message', + contains('maximum compressed size'), + )), + ); + }); + + test('rejects archives with too many entries', () { + final archive = Archive(); + for (var i = 0; i < 6; i++) { + archive.addFile(ArchiveFile('file$i.txt', 1, [i])); + } + final zipBytes = ZipEncoder().encode(archive)!; + + expect( + () => SafeZipExtractor.decodeBytes(zipBytes, limits: _tightLimits), + throwsA(isA().having( + (e) => e.message, + 'message', + contains('too many entries'), + )), + ); + }); + + test('rejects archives exceeding total uncompressed size', () { + const limits = ZipDecodeLimits( + maxCompressedBytes: 4096, + maxTotalUncompressedBytes: 6000, + maxEntryCount: 5, + maxEntryUncompressedBytes: 5000, + maxCompressionRatio: 100, + ); + final archive = Archive() + ..addFile(ArchiveFile('a.bin', 4000, List.filled(4000, 1))) + ..addFile(ArchiveFile('b.bin', 4000, List.filled(4000, 2))); + final zipBytes = ZipEncoder().encode(archive)!; + + expect( + () => SafeZipExtractor.decodeBytes(zipBytes, limits: limits), + throwsA(isA().having( + (e) => e.message, + 'message', + contains('total uncompressed size'), + )), + ); + }); + + test('rejects high compression ratio zip bombs', () { + const limits = ZipDecodeLimits( + maxCompressedBytes: 4096, + maxTotalUncompressedBytes: 8192, + maxEntryCount: 5, + maxEntryUncompressedBytes: 10000, + maxCompressionRatio: 10, + ); + final payload = List.filled(5000, 0); + final archive = _singleFileArchive('bomb.bin', payload); + final zipBytes = ZipEncoder().encode(archive)!; + + expect( + () => SafeZipExtractor.decodeBytes(zipBytes, limits: limits), + throwsA(isA().having( + (e) => e.message, + 'message', + contains('compression ratio'), + )), + ); + }); + + test('readAndDecodeFile reads bounded archives from disk', () async { + final archive = _singleFileArchive('ok.txt', utf8.encode('ok')); + final zipFile = await _writeZip(tempDir, archive, 'ok.zip'); + + final decoded = + await SafeZipExtractor.readAndDecodeFile(zipFile, limits: _tightLimits); + expect(decoded.first.name, 'ok.txt'); + }); + }); +} From 8913d6983c297277f891e5d28d75d227b5fd73d7 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Mon, 27 Jul 2026 06:47:50 +0300 Subject: [PATCH 2/2] chore(test): remove unnecessary null assertions in safe zip tests --- test/core/security/safe_zip_extractor_test.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/core/security/safe_zip_extractor_test.dart b/test/core/security/safe_zip_extractor_test.dart index 56fab3e7..283a4f63 100644 --- a/test/core/security/safe_zip_extractor_test.dart +++ b/test/core/security/safe_zip_extractor_test.dart @@ -19,7 +19,7 @@ Archive _singleFileArchive(String name, List content) { } Future _writeZip(Directory dir, Archive archive, String name) async { - final bytes = ZipEncoder().encode(archive)!; + final bytes = ZipEncoder().encode(archive); final file = File(p.join(dir.path, name)); await file.writeAsBytes(bytes); return file; @@ -41,7 +41,7 @@ void main() { test('decodes a small valid archive', () { final archive = _singleFileArchive('hello.txt', utf8.encode('hello')); - final zipBytes = ZipEncoder().encode(archive)!; + final zipBytes = ZipEncoder().encode(archive); final decoded = SafeZipExtractor.decodeBytes(zipBytes, limits: _tightLimits); expect(decoded.length, 1); @@ -67,7 +67,7 @@ void main() { for (var i = 0; i < 6; i++) { archive.addFile(ArchiveFile('file$i.txt', 1, [i])); } - final zipBytes = ZipEncoder().encode(archive)!; + final zipBytes = ZipEncoder().encode(archive); expect( () => SafeZipExtractor.decodeBytes(zipBytes, limits: _tightLimits), @@ -90,7 +90,7 @@ void main() { final archive = Archive() ..addFile(ArchiveFile('a.bin', 4000, List.filled(4000, 1))) ..addFile(ArchiveFile('b.bin', 4000, List.filled(4000, 2))); - final zipBytes = ZipEncoder().encode(archive)!; + final zipBytes = ZipEncoder().encode(archive); expect( () => SafeZipExtractor.decodeBytes(zipBytes, limits: limits), @@ -112,7 +112,7 @@ void main() { ); final payload = List.filled(5000, 0); final archive = _singleFileArchive('bomb.bin', payload); - final zipBytes = ZipEncoder().encode(archive)!; + final zipBytes = ZipEncoder().encode(archive); expect( () => SafeZipExtractor.decodeBytes(zipBytes, limits: limits),