From 2ce24398e4d42eaa26eb4edae6110080d9764ddc Mon Sep 17 00:00:00 2001 From: Dustin Date: Wed, 5 Aug 2026 22:54:27 +0200 Subject: [PATCH] Security-Review-Fixes (Builder): Salt-Regression behoben + Defense-in-Depth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Navidrome-Salt wieder kryptographisch sicher (Random.secure, 16 Bytes) - Regression: v2.31-Redesign (36c744d) hatte den Security-Audit-Fix aus 77a2042 stillschweigend auf Timestamp zurueckgesetzt - Server-IDs (Navidrome s.id, Registry sid) vor Dateinamen defensiv sanitized (sanitizeDateiname) — kein Path-Traversal ueber Server-Werte - MANAGE_EXTERNAL_STORAGE entfernt: deklariert aber nie angefragt (toter Berechtigungs-Surface, Play-Store-Policy-Risiko) - Neuer Test: navidrome_salt_test.dart (Salt != Timestamp, 16 Bytes, eindeutig) Verifiziert: flutter analyze 0 Issues, flutter test 156/156 --- android/app/src/main/AndroidManifest.xml | 1 - lib/screens/download_screen.dart | 4 ++- lib/services/navidrome_service.dart | 14 +++++++++-- test/navidrome_salt_test.dart | 32 ++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 test/navidrome_salt_test.dart diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 7d8fb4e..7208d24 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -6,7 +6,6 @@ - with WidgetsBindingObse try { final dir = Directory('${(await getApplicationDocumentsDirectory()).path}/music'); if (!await dir.exists()) await dir.create(recursive: true); - final dest = '${dir.path}/cloud_$sid.mp3'; + // Server-ID defensiv sanitizen — kein Path-Traversal über sid + final dest = '${dir.path}/cloud_${sanitizeDateiname(sid)}.mp3'; final ok = await _cloud.download(sid, dest); if (ok && mounted) { final song = Song( diff --git a/lib/services/navidrome_service.dart b/lib/services/navidrome_service.dart index 6fc78be..a662438 100644 --- a/lib/services/navidrome_service.dart +++ b/lib/services/navidrome_service.dart @@ -1,5 +1,6 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:math'; import 'package:flutter/foundation.dart'; import 'package:crypto/crypto.dart'; import 'package:http/http.dart' as http; @@ -81,7 +82,10 @@ class NavidromeService { _serverUrl = url.endsWith('/') ? url.substring(0, url.length - 1) : url; _user = user; _password = password; - _salt = DateTime.now().millisecondsSinceEpoch.toString(); + // Kryptographisch sicherer Salt (Random.secure statt vorhersagbarem + // Timestamp) — Security-Audit-Fix, war in v2.31-Redesign verloren gegangen. + final rng = Random.secure(); + _salt = base64Encode(List.generate(16, (_) => rng.nextInt(256))); _token = md5.convert(utf8.encode(_password + _salt)).toString(); } @@ -133,6 +137,10 @@ class NavidromeService { bool get istVerbunden => _serverUrl.isNotEmpty && _user.isNotEmpty; + /// Nur für Tests: Salt einsehbar, um die Salt-Qualität zu prüfen. + @visibleForTesting + String get saltFuerTests => _salt; + Uri _uri(String endpoint, [Map? extra]) { final params = { 'u': _user, @@ -203,7 +211,9 @@ class NavidromeService { if (!await musikDir.exists()) await musikDir.create(recursive: true); final safeName = sanitizeDateiname(s.titel); - final kurzId = s.id.length > 8 ? s.id.substring(0, 8) : s.id; + // Server-ID defensiv sanitizen (kein Path-Traversal über s.id) + final safeId = sanitizeDateiname(s.id); + final kurzId = safeId.length > 8 ? safeId.substring(0, 8) : safeId; final dateiName = '${safeName}_$kurzId.mp3'; final dateiPfad = '${musikDir.path}/$dateiName'; diff --git a/test/navidrome_salt_test.dart b/test/navidrome_salt_test.dart new file mode 100644 index 0000000..fc01518 --- /dev/null +++ b/test/navidrome_salt_test.dart @@ -0,0 +1,32 @@ +import 'dart:convert'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:melo_app/services/navidrome_service.dart'; + +void main() { + group('NavidromeService — Salt-Qualität (Security-Regression)', () { + test('Salt ist base64-codiert und KEIN vorhersagbarer Timestamp', () { + final nav = NavidromeService(); + nav.setCredentials('https://musik.example.de', 'user', 'pass'); + final salt = nav.saltFuerTests; + + // Timestamp-Salt (alte, unsichere Variante) wäre rein numerisch + expect(RegExp(r'^\d+$').hasMatch(salt), isFalse, + reason: 'Salt darf kein reiner Timestamp sein'); + + // base64-Decodierung muss 16 Zufallsbytes ergeben (128 Bit Entropie) + final bytes = base64Decode(salt); + expect(bytes.length, 16); + }); + + test('Zwei setCredentials-Aufrufe erzeugen unterschiedliche Salts', () { + final nav = NavidromeService(); + nav.setCredentials('https://musik.example.de', 'user', 'pass'); + final salt1 = nav.saltFuerTests; + nav.setCredentials('https://musik.example.de', 'user', 'pass'); + final salt2 = nav.saltFuerTests; + + // Kollisionswahrscheinlichkeit bei Random.secure (16 Byte) ≈ 0 + expect(salt1, isNot(equals(salt2))); + }); + }); +}