Skip to content

Commit 2617a81

Browse files
ZhuchkaTriplesixZhuchkaTriplesix
authored
Merge pull request #407 from QueryaHub/issue/398-safe-zip-extractor
fix(security): add SafeZipExtractor limits for archive installs
2 parents 24e700e + 8913d69 commit 2617a81

6 files changed

Lines changed: 318 additions & 7 deletions

File tree

‎docs/security.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,17 @@ On upgrade from older databases, existing plaintext secrets in SQLite are **migr
2121
## Tests
2222

2323
Automated tests use an **in-memory** secrets backend (see `test/flutter_test_config.dart`) so CI does not require a desktop keyring.
24+
25+
## Archive install limits (extensions and updates)
26+
27+
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:
28+
29+
| Limit | Default |
30+
|-------|---------|
31+
| Max compressed archive size | 100 MiB |
32+
| Max total uncompressed size | 500 MiB |
33+
| Max entries | 10 000 |
34+
| Max single entry uncompressed size | 100 MiB |
35+
| Max compression ratio (uncompressed ÷ compressed) | 100:1 |
36+
37+
Archives exceeding these bounds fail closed before files are written to disk. Path traversal checks remain in `archive_path_guard.dart`.

‎lib/core/extensions/local_extension_installer.dart‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import 'package:querya_desktop/core/extensions/models/extension_manifest.dart';
1111
import 'package:querya_desktop/core/extensions/sandbox/sandbox_policy.dart';
1212
import 'package:querya_desktop/core/market/marketplace_repository.dart';
1313
import 'package:querya_desktop/core/security/archive_path_guard.dart';
14+
import 'package:querya_desktop/core/security/safe_zip_extractor.dart';
1415

1516
/// Installs an extension package from a local `.zip` / `.qext` archive (issue #316).
1617
///
@@ -43,7 +44,12 @@ class LocalExtensionInstaller {
4344
}
4445

4546
onProgress?.call(0.1);
46-
final bytes = await archiveFile.readAsBytes();
47+
late final List<int> bytes;
48+
try {
49+
bytes = await SafeZipExtractor.readBoundedBytes(archiveFile);
50+
} on SafeZipException catch (error) {
51+
throw MarketplaceException(error.message);
52+
}
4753

4854
if (expectedSha256 != null && expectedSha256.trim().isNotEmpty) {
4955
final actual = sha256.convert(bytes).toString().toLowerCase();
@@ -57,7 +63,12 @@ class LocalExtensionInstaller {
5763
}
5864

5965
onProgress?.call(0.25);
60-
final archive = ZipDecoder().decodeBytes(bytes);
66+
late final Archive archive;
67+
try {
68+
archive = SafeZipExtractor.decodeBytes(bytes);
69+
} on SafeZipException catch (error) {
70+
throw MarketplaceException(error.message);
71+
}
6172
if (archive.isEmpty) {
6273
throw MarketplaceException('Extension archive is empty.');
6374
}

‎lib/core/market/http_marketplace_repository.dart‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:querya_desktop/core/extensions/local_extension_registry.dart';
1313
import 'package:querya_desktop/core/extensions/models/extension_manifest.dart';
1414
import 'package:querya_desktop/core/extensions/models/extension_type.dart';
1515
import 'package:querya_desktop/core/security/archive_path_guard.dart';
16+
import 'package:querya_desktop/core/security/safe_zip_extractor.dart';
1617
import 'marketplace_download_policy.dart';
1718
import 'marketplace_repository.dart';
1819

@@ -170,7 +171,12 @@ class HttpMarketplaceRepository implements MarketplaceRepository {
170171
);
171172
}
172173

173-
final bytes = await archiveFile.readAsBytes();
174+
late final List<int> bytes;
175+
try {
176+
bytes = await SafeZipExtractor.readBoundedBytes(archiveFile);
177+
} on SafeZipException catch (error) {
178+
throw MarketplaceException(error.message);
179+
}
174180
final actualSha256 = sha256.convert(bytes).toString().toLowerCase();
175181
if (actualSha256 != expectedSha256) {
176182
throw MarketplaceException(
@@ -181,8 +187,13 @@ class HttpMarketplaceRepository implements MarketplaceRepository {
181187

182188
onProgress?.call(0.85);
183189

184-
// Step 3: Safe Archive Extraction (Preventing Path Traversal / Zip Bomb - Issue #242)
185-
final archive = ZipDecoder().decodeBytes(bytes);
190+
// Step 3: Safe Archive Extraction (path traversal + zip bomb limits)
191+
final Archive archive;
192+
try {
193+
archive = SafeZipExtractor.decodeBytes(bytes);
194+
} on SafeZipException catch (error) {
195+
throw MarketplaceException(error.message);
196+
}
186197

187198
final dir = await ExtensionPaths.extensionsDirectory();
188199
final extDir = Directory(p.join(dir.path, manifest.id));

‎lib/core/security/safe_zip_extractor.dart‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import 'dart:io';
2+
3+
import 'package:archive/archive.dart';
4+
5+
/// Bounds for zip decode/extract to mitigate zip bombs and memory exhaustion.
6+
class ZipDecodeLimits {
7+
const ZipDecodeLimits({
8+
required this.maxCompressedBytes,
9+
required this.maxTotalUncompressedBytes,
10+
required this.maxEntryCount,
11+
required this.maxEntryUncompressedBytes,
12+
required this.maxCompressionRatio,
13+
});
14+
15+
final int maxCompressedBytes;
16+
final int maxTotalUncompressedBytes;
17+
final int maxEntryCount;
18+
final int maxEntryUncompressedBytes;
19+
final double maxCompressionRatio;
20+
21+
/// Default limits for marketplace, sideload, and updater archives.
22+
static const ZipDecodeLimits standard = ZipDecodeLimits(
23+
maxCompressedBytes: 100 * 1024 * 1024,
24+
maxTotalUncompressedBytes: 500 * 1024 * 1024,
25+
maxEntryCount: 10000,
26+
maxEntryUncompressedBytes: 100 * 1024 * 1024,
27+
maxCompressionRatio: 100,
28+
);
29+
}
30+
31+
/// Thrown when an archive exceeds [ZipDecodeLimits].
32+
class SafeZipException implements Exception {
33+
SafeZipException(this.message);
34+
35+
final String message;
36+
37+
@override
38+
String toString() => 'SafeZipException: $message';
39+
}
40+
41+
/// Bounded zip decode used by marketplace, sideload, and updater paths.
42+
abstract final class SafeZipExtractor {
43+
static Future<List<int>> readBoundedBytes(
44+
File file, {
45+
ZipDecodeLimits limits = ZipDecodeLimits.standard,
46+
}) async {
47+
final length = await file.length();
48+
if (length > limits.maxCompressedBytes) {
49+
throw SafeZipException(
50+
'Archive exceeds maximum compressed size '
51+
'(${limits.maxCompressedBytes} bytes).',
52+
);
53+
}
54+
return file.readAsBytes();
55+
}
56+
57+
static Archive decodeBytes(
58+
List<int> bytes, {
59+
ZipDecodeLimits limits = ZipDecodeLimits.standard,
60+
}) {
61+
if (bytes.length > limits.maxCompressedBytes) {
62+
throw SafeZipException(
63+
'Archive exceeds maximum compressed size '
64+
'(${limits.maxCompressedBytes} bytes).',
65+
);
66+
}
67+
68+
final Archive archive;
69+
try {
70+
archive = ZipDecoder().decodeBytes(bytes);
71+
} on Object catch (error) {
72+
throw SafeZipException('Failed to decode zip archive: $error');
73+
}
74+
75+
_validateArchive(
76+
archive,
77+
compressedBytes: bytes.length,
78+
limits: limits,
79+
);
80+
return archive;
81+
}
82+
83+
static Future<Archive> readAndDecodeFile(
84+
File file, {
85+
ZipDecodeLimits limits = ZipDecodeLimits.standard,
86+
}) async {
87+
final bytes = await readBoundedBytes(file, limits: limits);
88+
return decodeBytes(bytes, limits: limits);
89+
}
90+
91+
static void _validateArchive(
92+
Archive archive, {
93+
required int compressedBytes,
94+
required ZipDecodeLimits limits,
95+
}) {
96+
if (archive.length > limits.maxEntryCount) {
97+
throw SafeZipException(
98+
'Archive contains too many entries (${archive.length}; '
99+
'max ${limits.maxEntryCount}).',
100+
);
101+
}
102+
103+
var totalUncompressed = 0;
104+
for (final entry in archive) {
105+
if (!entry.isFile) continue;
106+
107+
final size = entry.size;
108+
if (size > limits.maxEntryUncompressedBytes) {
109+
throw SafeZipException(
110+
'Archive entry "${entry.name}" exceeds maximum uncompressed size '
111+
'($size bytes; max ${limits.maxEntryUncompressedBytes}).',
112+
);
113+
}
114+
115+
totalUncompressed += size;
116+
if (totalUncompressed > limits.maxTotalUncompressedBytes) {
117+
throw SafeZipException(
118+
'Archive exceeds maximum total uncompressed size '
119+
'(max ${limits.maxTotalUncompressedBytes} bytes).',
120+
);
121+
}
122+
}
123+
124+
if (compressedBytes > 0 && totalUncompressed > 0) {
125+
final ratio = totalUncompressed / compressedBytes;
126+
if (ratio > limits.maxCompressionRatio) {
127+
throw SafeZipException(
128+
'Archive compression ratio is too high '
129+
'(${ratio.toStringAsFixed(1)}:1; max ${limits.maxCompressionRatio}:1).',
130+
);
131+
}
132+
}
133+
}
134+
}

‎lib/core/updater/installers/update_install_utils.dart‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'package:archive/archive.dart';
44
import 'package:path/path.dart' as p;
55

66
import '../../security/archive_path_guard.dart';
7+
import '../../security/safe_zip_extractor.dart';
78
import '../app_updater_service.dart';
89

910
/// Safely extracts a zip archive into [destinationDir].
@@ -16,8 +17,12 @@ Future<void> extractZipSecurely({
1617
}
1718
await destinationDir.create(recursive: true);
1819

19-
final bytes = await zipFile.readAsBytes();
20-
final archive = ZipDecoder().decodeBytes(bytes);
20+
final Archive archive;
21+
try {
22+
archive = await SafeZipExtractor.readAndDecodeFile(zipFile);
23+
} on SafeZipException catch (error) {
24+
throw AppUpdaterException(error.message);
25+
}
2126
final root = p.normalize(destinationDir.path);
2227

2328
for (final entry in archive) {

‎test/core/security/safe_zip_extractor_test.dart‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import 'dart:convert';
2+
import 'dart:io';
3+
4+
import 'package:archive/archive.dart';
5+
import 'package:flutter_test/flutter_test.dart';
6+
import 'package:path/path.dart' as p;
7+
import 'package:querya_desktop/core/security/safe_zip_extractor.dart';
8+
9+
const _tightLimits = ZipDecodeLimits(
10+
maxCompressedBytes: 4096,
11+
maxTotalUncompressedBytes: 8192,
12+
maxEntryCount: 5,
13+
maxEntryUncompressedBytes: 4096,
14+
maxCompressionRatio: 10,
15+
);
16+
17+
Archive _singleFileArchive(String name, List<int> content) {
18+
return Archive()..addFile(ArchiveFile(name, content.length, content));
19+
}
20+
21+
Future<File> _writeZip(Directory dir, Archive archive, String name) async {
22+
final bytes = ZipEncoder().encode(archive);
23+
final file = File(p.join(dir.path, name));
24+
await file.writeAsBytes(bytes);
25+
return file;
26+
}
27+
28+
void main() {
29+
group('SafeZipExtractor', () {
30+
late Directory tempDir;
31+
32+
setUp(() async {
33+
tempDir = await Directory.systemTemp.createTemp('querya_safe_zip_');
34+
});
35+
36+
tearDown(() async {
37+
if (await tempDir.exists()) {
38+
await tempDir.delete(recursive: true);
39+
}
40+
});
41+
42+
test('decodes a small valid archive', () {
43+
final archive = _singleFileArchive('hello.txt', utf8.encode('hello'));
44+
final zipBytes = ZipEncoder().encode(archive);
45+
46+
final decoded = SafeZipExtractor.decodeBytes(zipBytes, limits: _tightLimits);
47+
expect(decoded.length, 1);
48+
expect(decoded.first.name, 'hello.txt');
49+
});
50+
51+
test('rejects archives exceeding max compressed bytes', () async {
52+
final file = File(p.join(tempDir.path, 'oversize.zip'));
53+
await file.writeAsBytes(List<int>.filled(5000, 1));
54+
55+
expect(
56+
() => SafeZipExtractor.readBoundedBytes(file, limits: _tightLimits),
57+
throwsA(isA<SafeZipException>().having(
58+
(e) => e.message,
59+
'message',
60+
contains('maximum compressed size'),
61+
)),
62+
);
63+
});
64+
65+
test('rejects archives with too many entries', () {
66+
final archive = Archive();
67+
for (var i = 0; i < 6; i++) {
68+
archive.addFile(ArchiveFile('file$i.txt', 1, [i]));
69+
}
70+
final zipBytes = ZipEncoder().encode(archive);
71+
72+
expect(
73+
() => SafeZipExtractor.decodeBytes(zipBytes, limits: _tightLimits),
74+
throwsA(isA<SafeZipException>().having(
75+
(e) => e.message,
76+
'message',
77+
contains('too many entries'),
78+
)),
79+
);
80+
});
81+
82+
test('rejects archives exceeding total uncompressed size', () {
83+
const limits = ZipDecodeLimits(
84+
maxCompressedBytes: 4096,
85+
maxTotalUncompressedBytes: 6000,
86+
maxEntryCount: 5,
87+
maxEntryUncompressedBytes: 5000,
88+
maxCompressionRatio: 100,
89+
);
90+
final archive = Archive()
91+
..addFile(ArchiveFile('a.bin', 4000, List<int>.filled(4000, 1)))
92+
..addFile(ArchiveFile('b.bin', 4000, List<int>.filled(4000, 2)));
93+
final zipBytes = ZipEncoder().encode(archive);
94+
95+
expect(
96+
() => SafeZipExtractor.decodeBytes(zipBytes, limits: limits),
97+
throwsA(isA<SafeZipException>().having(
98+
(e) => e.message,
99+
'message',
100+
contains('total uncompressed size'),
101+
)),
102+
);
103+
});
104+
105+
test('rejects high compression ratio zip bombs', () {
106+
const limits = ZipDecodeLimits(
107+
maxCompressedBytes: 4096,
108+
maxTotalUncompressedBytes: 8192,
109+
maxEntryCount: 5,
110+
maxEntryUncompressedBytes: 10000,
111+
maxCompressionRatio: 10,
112+
);
113+
final payload = List<int>.filled(5000, 0);
114+
final archive = _singleFileArchive('bomb.bin', payload);
115+
final zipBytes = ZipEncoder().encode(archive);
116+
117+
expect(
118+
() => SafeZipExtractor.decodeBytes(zipBytes, limits: limits),
119+
throwsA(isA<SafeZipException>().having(
120+
(e) => e.message,
121+
'message',
122+
contains('compression ratio'),
123+
)),
124+
);
125+
});
126+
127+
test('readAndDecodeFile reads bounded archives from disk', () async {
128+
final archive = _singleFileArchive('ok.txt', utf8.encode('ok'));
129+
final zipFile = await _writeZip(tempDir, archive, 'ok.zip');
130+
131+
final decoded =
132+
await SafeZipExtractor.readAndDecodeFile(zipFile, limits: _tightLimits);
133+
expect(decoded.first.name, 'ok.txt');
134+
});
135+
});
136+
}

0 commit comments

Comments
 (0)