From 40d1c416984b5835d60752c35e9556c740e6089c Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 17 Oct 2026 05:40:32 +0300 Subject: [PATCH] fix(security): zip limits cannot be bypassed by a header that understates sizes (#1561) SafeZipExtractor checks its limits against the uncompressed sizes the zip headers declare. Before archive 4.5.0 that size was only a buffer hint and an entry decoded to whatever its data held, so a crafted header bypassed every limit. Since 4.5.0 an entry is not decoded past its declared size. - Require archive ^4.5.2 (pubspec.lock is not committed, so the constraint is what CI and release builds resolve against). - SafeZipExtractor.readEntry reads an entry's bytes and reports the refusal as a SafeZipException; the local installer, the HTTP marketplace and the updater extract through it. - Tests: a zip whose headers claim 100 bytes for 64 KiB passes the header checks and is refused when read, from bytes and from a file. --- CHANGELOG.md | 1 + .../extensions/local_extension_installer.dart | 9 ++- .../market/http_marketplace_repository.dart | 7 +- lib/core/security/safe_zip_extractor.dart | 19 +++++ .../installers/update_install_utils.dart | 9 ++- pubspec.yaml | 4 +- .../security/safe_zip_extractor_test.dart | 69 +++++++++++++++++++ 7 files changed, 114 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f44a50..1dbca856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Zip limits hold even when an archive lies about its sizes** — the per-entry, total and ratio limits for extension installs, sideloads and updates are checked against the sizes the zip headers declare; the app now requires `archive` 4.5+, which refuses to decode an entry past its declared size, and reports that as an error instead of inflating it (#1561). - **`querya-ext-tester` sends a `--connect` password with `system.injectCredentials`, like the app** — a driver that reads its password only from the injected secret (the ClickHouse driver) failed authentication under the tester because the password was in the `db.connect` payload only. - **Demo Marketplace themes were named `${manifest.name}`** — the generated theme.json now carries the real name (the demo catalog is only used with `--dart-define=QUERYA_MARKETPLACE_MOCK=true`). - **In-app update of a package install (AUR, `.deb`, `.rpm`)** — the updater no longer downloads a zip and quits when the app lives in `/opt/querya-desktop` (owned by root): the update dialog says how to update it (`yay -Syu querya-desktop`, `apt`, `dnf`) with a *Copy update command* button, or offers the release page for any other folder the user cannot write. If an in-place replace fails anyway, the app is started again and the reason is logged to `~/.local/state/querya-update.log`, instead of staying closed. diff --git a/lib/core/extensions/local_extension_installer.dart b/lib/core/extensions/local_extension_installer.dart index 1afaafa6..eee54d8d 100644 --- a/lib/core/extensions/local_extension_installer.dart +++ b/lib/core/extensions/local_extension_installer.dart @@ -1,5 +1,6 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:typed_data'; import 'package:archive/archive.dart'; import 'package:path/path.dart' as p; @@ -260,7 +261,13 @@ class LocalExtensionInstaller { if (file.isFile) { final outFile = File(targetPath); await outFile.parent.create(recursive: true); - await outFile.writeAsBytes(file.content); + final Uint8List bytes; + try { + bytes = SafeZipExtractor.readEntry(file); + } on SafeZipException catch (error) { + throw MarketplaceException(error.message); + } + await outFile.writeAsBytes(bytes); file.clear(); } else { await Directory(targetPath).create(recursive: true); diff --git a/lib/core/market/http_marketplace_repository.dart b/lib/core/market/http_marketplace_repository.dart index 164674ea..bdc8d317 100644 --- a/lib/core/market/http_marketplace_repository.dart +++ b/lib/core/market/http_marketplace_repository.dart @@ -225,7 +225,12 @@ class HttpMarketplaceRepository implements MarketplaceRepository { if (file.isFile) { final outFile = File(targetPath); await outFile.create(recursive: true); - final bytes = file.content; + final Uint8List bytes; + try { + bytes = SafeZipExtractor.readEntry(file); + } on SafeZipException catch (error) { + throw MarketplaceException(error.message); + } await outFile.writeAsBytes(bytes); file.clear(); } else { diff --git a/lib/core/security/safe_zip_extractor.dart b/lib/core/security/safe_zip_extractor.dart index 17d05759..8f1deeca 100644 --- a/lib/core/security/safe_zip_extractor.dart +++ b/lib/core/security/safe_zip_extractor.dart @@ -1,4 +1,5 @@ import 'dart:io'; +import 'dart:typed_data'; import 'package:archive/archive.dart'; @@ -122,6 +123,24 @@ abstract final class SafeZipExtractor { } } + /// The decoded bytes of [entry], from an archive returned by [decodeBytes] + /// or [readAndDecodeFile]. + /// + /// The limits are checked against the sizes the zip headers declare. That + /// holds because the archive package (4.5.0+) refuses to decode an entry + /// past its declared size, so a header that understates it cannot inflate + /// to more than was checked (#1561). That refusal is a [SafeZipException]. + static Uint8List readEntry(ArchiveFile entry) { + try { + return entry.content; + } on ArchiveException catch (error) { + throw SafeZipException( + 'Archive entry "${entry.name}" is larger than its header declares ' + '(${error.message}).', + ); + } + } + static void _validateArchive( Archive archive, { required int compressedBytes, diff --git a/lib/core/updater/installers/update_install_utils.dart b/lib/core/updater/installers/update_install_utils.dart index ff0489c1..a536ff74 100644 --- a/lib/core/updater/installers/update_install_utils.dart +++ b/lib/core/updater/installers/update_install_utils.dart @@ -1,4 +1,5 @@ import 'dart:io'; +import 'dart:typed_data'; import 'package:archive/archive.dart'; import 'package:path/path.dart' as p; @@ -43,7 +44,13 @@ Future extractZipSecurely({ if (entry.isFile) { final out = File(targetPath); await out.parent.create(recursive: true); - await out.writeAsBytes(entry.content as List); + final Uint8List bytes; + try { + bytes = SafeZipExtractor.readEntry(entry); + } on SafeZipException catch (error) { + throw AppUpdaterException(error.message); + } + await out.writeAsBytes(bytes); } else { await Directory(targetPath).create(recursive: true); } diff --git a/pubspec.yaml b/pubspec.yaml index 1a80a8f5..ee61ffc8 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -30,7 +30,9 @@ dependencies: flutter_secure_storage: ^10.3.1 file_selector: ^1.1.0 syntax_highlight: ^0.5.0 - archive: ^4.2.0 + # 4.5.0+: a zip entry is not decoded past the size its header declares, + # which SafeZipExtractor's limits rely on (#1561). + archive: ^4.5.2 url_launcher: ^6.3.1 package_info_plus: ^9.0.1 flutter_svg: ^2.3.0 diff --git a/test/core/security/safe_zip_extractor_test.dart b/test/core/security/safe_zip_extractor_test.dart index 6d9fc28e..6cb14216 100644 --- a/test/core/security/safe_zip_extractor_test.dart +++ b/test/core/security/safe_zip_extractor_test.dart @@ -1,5 +1,6 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:typed_data'; import 'package:archive/archive.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -25,6 +26,23 @@ Future _writeZip(Directory dir, Archive archive, String name) async { return file; } +/// [zip] with the uncompressed size of every entry rewritten to [size], in +/// the local headers and the central directory: a zip that lies about how +/// large its entries are. +Uint8List _withDeclaredSize(List zip, int size) { + final bytes = Uint8List.fromList(zip); + final data = ByteData.sublistView(bytes); + for (var i = 0; i + 30 <= bytes.length; i++) { + switch (data.getUint32(i, Endian.little)) { + case 0x04034b50: // local file header + data.setUint32(i + 22, size, Endian.little); + case 0x02014b50: // central directory header + data.setUint32(i + 24, size, Endian.little); + } + } + return bytes; +} + void main() { group('SafeZipExtractor', () { late Directory tempDir; @@ -133,6 +151,57 @@ void main() { expect(decoded.first.name, 'ok.txt'); }); + group('a header that understates the entry size (#1561)', () { + // 64 KiB of zeros deflate to a few hundred bytes; the header claims 100, + // so every header-based limit and the ratio check pass. + Uint8List lyingZip() => _withDeclaredSize( + ZipEncoder().encode( + _singleFileArchive('bomb.bin', Uint8List(64 * 1024)), + ), + 100, + ); + + test('passes the header checks', () { + final decoded = + SafeZipExtractor.decodeBytes(lyingZip(), limits: _tightLimits); + expect(decoded.first.size, 100); + }); + + test('readEntry stops at the declared size', () { + final decoded = + SafeZipExtractor.decodeBytes(lyingZip(), limits: _tightLimits); + expect( + () => SafeZipExtractor.readEntry(decoded.first), + throwsA(isA().having( + (e) => e.message, + 'message', + contains('larger than its header declares'), + )), + ); + }); + + test('readEntry stops at the declared size when read from a file', + () async { + final file = File(p.join(tempDir.path, 'lying.zip')); + await file.writeAsBytes(lyingZip()); + final decoded = + await SafeZipExtractor.readAndDecodeFile(file, limits: _tightLimits); + expect( + () => SafeZipExtractor.readEntry(decoded.first), + throwsA(isA()), + ); + }); + }); + + test('readEntry returns the content of an honest entry', () { + final zipBytes = ZipEncoder().encode( + _singleFileArchive('hello.txt', utf8.encode('hello')), + ); + final decoded = + SafeZipExtractor.decodeBytes(zipBytes, limits: _tightLimits); + expect(utf8.decode(SafeZipExtractor.readEntry(decoded.first)), 'hello'); + }); + test('ensureCompressedSizeAllowed rejects oversize before decode', () async { final file = File(p.join(tempDir.path, 'big.zip')); await file.writeAsBytes(List.filled(5000, 1));