Files
RouterOS/bugs.md
T
KayandClaude Sonnet 5 aad2ccb7d4 Härtung: REST-Query-Injection-Parität + TOFU-Fingerprint-Fallback
Dritter, sicherheitsfokussierter Deep-Dive-Durchgang ("maximale Sicherheit"):

- RestTransport.fetchFieldValues hatte dieselbe ungeschützte
  String-Interpolation wie das SSH-Pendant aus dem vorigen Fix, nur als
  URL-Query statt CLI-Zeile - beim ersten Fix übersehen. Jetzt
  RFC-3986-konform percent-encoded.
- CertificateFingerprint.sha256 fiel bei Extraktionsfehlern auf einen
  festen String "unbekannt" zurück statt echtem Fingerabdruck -
  theoretisches TOFU-Pinning-Bypass-Fenster (zwei verschiedene,
  extraktions-fehlschlagende Zertifikate hätten sich denselben
  "Fingerabdruck" geteilt). Rückgabetyp optional, Extraktionsfehler
  führt jetzt zu hartem Verbindungsabbruch statt Trust-Dialog.

Beide Fixes defensiv/gehärtet, nicht live exploitiert. Zugangsdaten-
Speicherung (Keychain) und BackupServices eigene Escaping-Logik
gegengeprüft - bereits korrekt. Build + alle 101 Unit-Tests grün.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 18:14:44 +02:00

17 KiB

bugs.md

Ergebnis eines systematischen Tester-/Senior-Dev-Durchgangs (2026-09-17) gegen den echten hAP lite Testrouter (192.168.88.1, RouterOS 7.24.4). Hinweis zur Methode: die App-UI selbst konnte nicht geklickt werden (keine macOS-UI-Automatisierung verfügbar) — stattdessen Code-Audit jedes Feature-Bereichs, Router-Ground-Truth per SSH gegenprüft (/export terse, Live-Konfiguration), und wo möglich per Skript verifiziert statt geraten (siehe je Eintrag). Router-Zustand zum Testzeitpunkt: nahezu Werkszustand, nur die WLAN-Testkonfiguration aus einer früheren Session vorhanden.

Status: offen | postponed | fixed


2026-09-17

1. Race Condition bei Port-Konflikt-Prüfung im LAN-Schritt

Status: fixed (Build grün, noch nicht live gegenreproduziert) Confidence: hoch (Logikfehler direkt im Code nachvollzogen, nicht live reproduziert)

SetupViewModel.checkPortConflict(for:) (SetupViewModel.swift:146) liest config.interfaceName synchron beim Aufruf und startet dann einen Task, der das Ergebnis nach dem await unbedingt in lanPortConflicts[configID] schreibt — ohne Generation-Counter oder Abbruch des vorherigen Task.

Der Aufruf passiert an zwei Stellen für dieselbe configID: .onAppear (bei jedem Erscheinen des LAN-Schritts) und .onChange(of: config.interfaceName) (bei jeder Port-Auswahl). Wechselt der Nutzer den Port zügig zweimal hintereinander (oder wechselt ihn, bevor die onAppear-Prüfung des vorherigen Ports fertig ist), können zwei Tasks parallel laufen. Der zuerst gestartete, aber zuletzt fertige Task überschreibt das Ergebnis des neueren mit dem Stand des alten Ports — "last response wins" statt "last request wins".

Konkretes Fehlerbild: Nutzer wählt Port A (hat Konflikt, z.B. Bridge-Mitgliedschaft), App prüft noch, Nutzer wechselt schnell zu Port B (frei). Kommt Port As Prüfergebnis später zurück als Port Bs, zeigt die App fälschlich eine Konflikt-Warnung für den inzwischen ausgewählten, tatsächlich freien Port B — oder, im umgekehrten Fall, verschluckt eine echte Warnung für einen Port, der tatsächlich bereits belegt ist, sodass "Weiter" freigeschaltet wird, obwohl der gewählte Port beim Anwenden unbemerkt vorhandene Konfiguration überschreibt.

Fix: Generation-Zähler portConflictRequestGeneration: [LanDhcpConfig.ID: Int] ergänzt, bei jedem checkPortConflict(for:)-Aufruf hochgezählt; ein abgeschlossener Task schreibt sein Ergebnis nur, wenn seine Generation beim Abschluss noch die aktuellste ist — ein überholter Task verwirft sein Ergebnis stillschweigend statt es zu übernehmen.

2. "Firewall (optional)"-Titel widerspricht sich selbst im Einfach-Modus

Status: fixed (Build grün, noch nicht live gegenreproduziert) Confidence: hoch (direkt im Code sichtbar)

FirewallStepView.swift:63: .navigationTitle(... "Firewall (optional)" ...) ist fest, unabhängig vom Modus. Im Einfach-Modus zeigt derselbe Screen aber den Text "Firewall-Grundschutz ist im einfachen Modus immer aktiv." (Zeile 20) — der Titel behauptet "optional", der Inhalt sagt "immer an". Für Experte-Modus stimmt der Titel (dort gibt es den Toggle). Kleiner, aber sofort sichtbarer Text-Widerspruch für jeden, der im Einfach-Modus durch den Wizard geht.

Fix: Titel modusabhängig gemacht — viewModel.mode == .expert ? "Firewall (optional)" : "Firewall", neuer Key "Firewall" in L10n.swift ergänzt.

3. Fehlende Englisch-Übersetzungen (sichtbar im EN-UI)

Status: fixed (skriptgeprüft: beide Keys jetzt in L10n.swift, verbleibende zwei fehlende Keys "OK"/"Revision" bewusst nicht ergänzt — identisch in beiden Sprachen, kein sichtbarer Effekt) Confidence: hoch (skriptgeprüft: alle L10n.t(...)-Aufrufstellen gegen L10n.swifts Übersetzungs-Dictionary abgeglichen — L10n.t fällt bei fehlendem Key auf den deutschen Originaltext zurück, siehe L10n.swift:12-15)

Zwei Stellen zeigen im Sprachmodus Englisch weiterhin deutschen Text:

  • Features/Devices/DevicesView.swift:335"Netzwerk-Test fehlgeschlagen" (Fehlermeldungstitel)
  • Features/Overview/OverviewView.swift:350"Fokus-Ansicht schließen" (Tooltip auf dem Schließen-Button der Fokus-Ansicht)

(Ein dritter fehlender Key, "Revision", ist praktisch nicht sichtbar, da das Wort in beiden Sprachen identisch ist — nicht extra gelistet.)

Fix-Ansatz: Beide Strings in L10n.swifts translations-Dictionary ergänzen ("Netzwerk-Test fehlgeschlagen": "Network test failed", "Fokus-Ansicht schließen": "Close focus view").

4. Health-Check-Heartbeat ignoriert laufende Wizard-/Experte-Schreibvorgänge

Status: fixed (Build grün, noch nicht live gegenreproduziert)

Fix: ConnectionService.beginWrite()/endWrite() (Zähler activeWriteCount) ergänzt, checkConnectionHealthAndReconnectIfNeeded() prüft jetzt zusätzlich activeWriteCount == 0. SetupViewModel.apply() und ExpertViewModel.saveEditingItem()/confirmRemoval() klammern ihren Task-Body jetzt mit beginWrite()/defer { endWrite() }. Confidence: mittel (Logiklücke im Code nachvollzogen; nicht live reproduziert — REST läuft über unabhängige HTTP-Requests und dürfte robust sein, SSH-Exec-Kanäle sind laut SSH-Protokoll grundsätzlich nebenläufig nutzbar, siehe SSHTransport.swift/RestTransport.swift — insofern kein Daten-Korruptionsrisiko, aber eine echte Lücke in der Ablaufsteuerung)

ConnectionService.checkConnectionHealthAndReconnectIfNeeded() (ConnectionService.swift:273) läuft alle 10 Sekunden (healthCheckInterval) und prüft nur !isReconnecting — nicht, ob gerade ein SetupViewModel.apply() oder ExpertViewModel.saveEditingItem() in Arbeit ist. Der hAP-lite- Testrouter ist sehr schwach (MIPS 24Kc, 1 Kern, 650MHz, im Test bereits 54% CPU-Last im Leerlauf gemessen). Ein mehrere Sekunden dauernder Apply (z.B. Bonding, mehrere Firewall-Regeln) kann zeitlich mit dem Heartbeat kollidieren; schlägt der Heartbeat unter Last mit Timeout fehl, während der eigentliche Apply eigentlich noch normal durchläuft, zeigt die App fälschlich den "Wiederverbinden…"-Banner an, obwohl gar keine echte Verbindungsunterbrechung vorliegt — und reconnectLoop könnte im ungünstigsten Fall sogar finishConnecting mit einer neuen Transport-Instanz auslösen, während der ursprüngliche Apply noch auf der alten weiterläuft.

Fix-Ansatz: Einfachster Schutz: SetupViewModel.isApplying / ExpertViewModel.isApplying (oder ein neuer gemeinsamer "isWriting"-Zähler auf ConnectionService) zusätzlich zur Guard-Bedingung in checkConnectionHealthAndReconnectIfNeeded() prüfen, damit der Heartbeat während eines aktiven Schreibvorgangs aussetzt.


2026-09-17 (Nachtrag: "erneuter Gegencheck mit echtem Deep Dive")

Zweite, tiefere Testrunde auf expliziten Nutzerwunsch. Diesmal inkl. echter Exploit-Verifikation gegen den Router (nicht nur Code-Lesen) — beide folgenden Funde live nachgewiesen und live gegenverifiziert, dass der Fix greift, mit anschließendem Aufräumen der Testartefakte.

5. RouterOS-CLI-Injection über beliebige Textfelder (SSH-Transportpfad)

Status: fixed (live exploitiert UND live als behoben bestätigt, siehe unten) Confidence: sehr hoch — kein Verdacht, sondern reproduzierter Exploit gegen den echten Router

RouterOSCommand.cliLine (RouterOSCommand.swift) baute CLI-Zeilen für den SSH-Transport per String-Interpolation. Die alte quoteIfNeeded(_:) quotete einen Wert nur, wenn er ein Leerzeichen enthielt, und escapte darin enthaltene "-Zeichen nie. RouterOS' Konsole behandelt ; als Befehlstrenner (dieselbe Gefahr, die SSHTransport.runDiagnosticCommands Doku-Kommentar für eigene, aufruferseitig sanitisierte Eingaben bereits benennt) — ein Wert wie test" ; :log warning "INJECTED (ein völlig plausibler freier Kommentar/SSID/Hostname) schloss das Anführungszeichen vorzeitig und ließ den Rest als zweiten, unabhängigen RouterOS-Befehl laufen.

Exploit live bestätigt (Testrouter, aufgeräumt danach): Befehl /ip firewall address-list add list=injection-test address=10.10.10.10 comment="test" ; :log warning "INJECTED-VIA-COMMENT-FIELD" über SSH ausgeführt → Log zeigt script,warning INJECTED-VIA-COMMENT-FIELD als eigenständig ausgeführten zweiten Befehl, ausgelöst rein durch ein Kommentarfeld.

Betroffen: jeder Text, der über den SSH-Transport in ein RouterOSCommand läuft — praktisch jedes Feld im Setup-Wizard (SSID, Kommentare, DNS-Server, …) und jedes Freitextfeld im Experte-Tab ("Weitere Parameter"). Der REST-Pfad ist NICHT betroffen (JSON-Encoding, kein String-Interpolieren — siehe RestTransport.apply). Auf dem aktuellen hAP-lite-Testrouter ist www-ssl deaktiviert (/ip service print bestätigt: Zeile 9, X = disabled) — REST ist also gar nicht erreichbar, jede Schreiboperation läuft aktuell über SSH. Der Bug war damit live aktiv, nicht nur theoretisch.

Fix: quoteIfNeeded quotet jetzt immer und escaped \\\, "\" innerhalb der Anführungszeichen — live verifiziert, dass das RouterOS' eigene Escape-Syntax ist (\"/\\ rundeten korrekt durch print detail). Unconditionelles Quoting live als unproblematisch bestätigt (auch für einwortige Werte und yes/no-Booleans anstandslos akzeptiert). Gleiche Lücke defensiv auch in SSHTransport.fetchFieldValues geschlossen (aktuell nur mit hartkodiertem "no" aufgerufen, aber generische RouterOSTransport-Methode). Exploit-PoC danach erneut gegen den Router gefahren — kein injizierter Log-Eintrag mehr, Kommentar korrekt als reiner Text gespeichert. Neuer Regressionstest testCliLineEscapesEmbeddedQuotesPreventingCommandInjection. Build + alle 101 Unit-Tests grün.

6. Genereller RouterOS-Antwort-Parser trunkiert mehrwortige Werte

Status: fixed (Ursache live nachgewiesen, Fix per Unit-Test abgesichert) Confidence: sehr hoch — live gegen zwei unabhängige Menüs nachgewiesen

RouterOSCliParser.keyValues(from:) nahm an, print terse quote mehrwortige Werte in Anführungszeichen (Regex ("[^"]*"|\S+)). Live-Test (zwei unabhängige Menüs, /ip firewall address-list und /interface ethernet, RouterOS 7.24.4) zeigt: RouterOS quotet dort gar nichts — ein Kommentar "multi word test value" kommt als literales, unquotiertes comment=multi word test value name=ether2 ... zurück. Der alte Regex-Parser matchte dafür nur \S+ und schnitt den Wert beim ersten Leerzeichen ab — "word test value" ging komplett verloren, ohne Fehler, ohne Warnung.

Betrifft praktisch jedes mehrwortige Freitextfeld, das über den generischen Parser gelesen wird — vor allem das Experte-Tab-"Weitere Parameter (frei)"-Grid, das laut eigenem Doku-Kommentar "fast alle" Menüs generisch parst. Der Bug war bereits indirekt im eigenen Testcode sichtbar: der existierende Live-Dump-Test testParseInterfacesMatchesLiveHexOutput enthält exakt dieses Muster (last-link-up-time=2026-09-15 20:19:19, ein echter Gerätedump) — nur fiel es nie auf, weil dieses Feld von keinem kuratierten Schema gelesen wird.

Fix: keyValues von Regex-Matching auf Token-Scanning umgestellt — ein Leerzeichen-getrenntes Wort startet nur dann ein neues Feld, wenn es selbst wie key=... aussieht; alles andere wird an den Wert des zuletzt begonnenen Feldes angehängt. Bekannter Restfall (inhärent durch RouterOS' mehrdeutiges unquotiertes Format, nicht clientseitig lösbar): ein Wert, der selbst ein wort=-Muster enthält (z.B. ein Kommentar "config=broken"), wird fälschlich als neues Feld gelesen — deutlich seltener als die vorherige 100%-Fehlerquote bei jedem mehrwortigen Wert. Neuer Regressionstest testParseGenericItemsPreservesUnquotedMultiWordValue, abgeleitet vom echten Live-Dump. Build + alle 101 Unit-Tests grün.

7. Beobachtung (kein Fix): Isolation wirkt nicht rückwirkend auf bereits bestehende Verbindungen

Status: offen (bewusst nicht automatisch gefixt — Risikoabwägung, siehe unten) Confidence: hoch (Regel-Reihenfolge im Code nachvollzogen), nicht live reproduziert

FirewallConfig.buildCommands() (FirewallConfig.swift) setzt die Regel "forward established,related → accept" auf Position 4, die Isolations-Drop-Regeln erst ab Position filterCommands.count (7+). Für eine neue Verbindung zwischen zwei isolierten Netzen ist das korrekt (erstes Paket hat connection-state=new, trifft also nicht Regel 4, sondern die Isolations-Regel weiter hinten). Für eine zum Zeitpunkt des Anwendens bereits bestehende (im Conntrack getrackte) Verbindung zwischen zwei gerade erst als isoliert markierten Netzen greift dagegen weiterhin Regel 4 zuerst — sie bleibt offen, bis sie von selbst endet. Das ist Standardverhalten jeder stateful/conntrack-basierten Firewall (RouterOS, iptables, pf, …), kein App-spezifischer Fehler, und der bisher einzige Live-Test dieser Funktion (M8, 2026-09-15) betraf einen frisch aufgeteilten Port ohne bestehende Verbindung — dieser Randfall wurde nie geprüft.

Nicht automatisch gefixt: ein Fix würde bedeuten, beim Aktivieren der Isolation gezielt /ip firewall connection remove für die betroffenen Netzpaare auszulösen — ein zusätzlicher, potenziell überraschender Seiteneffekt (kappt aktive Verbindungen, die der Nutzer evtl. bewusst offen hat), der über die reine Bugfix-Aufgabe dieses Durchgangs hinausgeht und eine bewusste Produktentscheidung ist, keine reine Korrektur. Nur als Beobachtung vermerkt.


2026-09-17 (Nachtrag 2: "finale Test für maximale Sicherheit")

Dritter Durchgang, gezielt sicherheitsfokussiert: Zugangsdaten-Speicherung (Keychain, SavedRoutersStore), TOFU-Implementierung (Zertifikat/SSH-Hostkey), alle verbleibenden Stellen, die Text in Router-Befehle oder URLs interpolieren. Positiv: KeychainService speichert Passwörter korrekt im macOS-Keychain (nie Klartext/UserDefaults), SavedRoutersStore/SavedRouter enthalten kein Passwort-Feld, BackupService.escapeForRouterOSScript escapte bereits vor diesem Durchgang korrekt (unabhängig von Fund #5 — offenbar aus einem früheren Lockout-Vorfall gelernt), SSHHostKeyFingerprint hat keine Fallback-Schwäche. Zwei weitere Lücken gefunden und gehärtet (defensiv, nicht live exploitiert):

8. REST-Pendant zu Fund #5: ungeschützte Query-String-Interpolation

Status: fixed (Build grün, nicht live exploitiert — aktuell nur mit hartkodiertem Wert aufgerufen) Confidence: hoch (dieselbe Bugklasse wie #5, nur im REST- statt SSH-Pfad)

RestTransport.fetchFieldValues baute den Query-String "\(restPath)?\(whereField)=\(whereValue)" ohne jede Kodierung — beim SSH-Gegenstück wurde das in Fund #5 bereits gefixt, das REST-Pendant dabei übersehen. Ein whereValue mit & hätte einen zusätzlichen Query-Parameter einschleusen können. Aktuell nur mit hartkodiertem "no" aufgerufen (kein Live-Exploit möglich), aber generische RouterOSTransport-Methode — Härtung für zukünftige Aufrufer mit echtem Nutzer-/Gerätetext.

Fix: whereValue wird jetzt RFC-3986-"unreserved characters"-only percent-encoded (strenger als .urlQueryAllowed, das &/=/+/# weiterhin durchlässt).

9. TOFU-Zertifikatsprüfung hatte einen Fallback-Konstante-Blindfleck

Status: fixed (Build grün, Härtung — kein realistisch auslösbarer Live-Exploit gefunden) Confidence: mittel (theoretische Lücke im Code nachvollzogen, keine funktionierende PoC gebaut, da eine echte TLS-Handshake-Situation gebraucht würde, in der SecTrustCopyCertificateChain trotz abgeschlossenem Handshake leer zurückkommt — unüblich, aber laut Apple-API-Vertrag nicht ausgeschlossen)

CertificateFingerprint.sha256(of:) gab bei fehlgeschlagener Zertifikatsketten-Extraktion den festen String "unbekannt" statt eines echten Fingerabdrucks zurück. Da CertificateTrustStore.isTrusted Fingerabdrücke per exaktem String-Vergleich prüft, hätte das Vertrauen in EIN Zertifikat, dessen Extraktion fehlschlägt, automatisch JEDES andere ebenfalls extraktions-fehlschlagende Zertifikat für denselben Host als vertrauenswürdig durchgehen lassen — ein theoretisches TOFU-Pinning-Bypass-Fenster.

Fix: Rückgabetyp auf String? geändert; schlägt die Extraktion fehl, lehnt RestTransport.urlSession(_:didReceive:completionHandler:) die Challenge jetzt hart ab (.cancelAuthenticationChallenge, kein lastRejectedFingerprint gesetzt) — der Nutzer bekommt in diesem Fall nie einen "Zertifikat vertrauen?"-Dialog mit einer nicht verifizierbaren Kennung angeboten, sondern einen normalen Verbindungsfehler.

Build + alle 101 Unit-Tests grün nach beiden Fixes.


Noch nicht geprüft / außerhalb dieses Durchgangs

  • UI-Interaktion selbst (Klickpfade, Darstellung) — nicht automatisierbar, siehe Hinweis oben.
  • PPPoE-Client — laut Nutzer am Testrouter nicht testbar (hängt hinter einem weiteren konfigurierten Router, kein direkter ISP-Uplink). Kein neuer Fund, nur zur Vollständigkeit.
  • Restliche Tabs/Bereiche (Übersicht-Diagramm-Interaktion, Sicherungen, Mode-Taste, Settings) wurden im Code überflogen, ohne konkreten neuen Fund über die bereits in found.md/HANDOFF.md dokumentierten Punkte hinaus.