From be0b38f64bf7c6795682d2faa3aacbd2b70cd289 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Fri, 25 Sep 2026 09:10:19 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20#213:=20KI-Vorschlag-Badge=20ohne=20inner?= =?UTF-8?q?HTML=20f=C3=BCr=20Kontobezeichnungen=20(Stored=20XSS)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 6 ++ public/static/css/app.css | 8 +++ public/static/js/app.js | 8 ++- public/static/js/beleg-ki.js | 77 +++++++++++++++++--------- src/Views/bank/show.php | 6 +- src/Views/belege/list.php | 6 +- src/Views/dashboard.php | 6 +- src/Views/settings/form.php | 6 +- tests/102_KiBadgeXssTest.php | 75 +++++++++++++++++++++++++ tests/e2e/specs/beleg-ki-badge.spec.js | 74 +++++++++++++++++++++++++ 10 files changed, 241 insertions(+), 31 deletions(-) create mode 100644 tests/102_KiBadgeXssTest.php create mode 100644 tests/e2e/specs/beleg-ki-badge.spec.js diff --git a/Changes.md b/Changes.md index e067190..5aa5c04 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,12 @@ --- +## 2026-09-25 – Stored XSS im KI-Vorschlag-Badge behoben (#213) + +- **`public/static/js/beleg-ki.js`:** `_showKiBadgeDone()` baut das Badge jetzt per `createElement`/`textContent` statt `innerHTML`. Kontobezeichnungen (aus `option.text`, also bereits dekodiert), Buchungstyp, USt-Satz und Lieferant landen nur noch als Text im DOM – eine Kontobezeichnung wie `Bank` wird nicht mehr ausgeführt. Der „Neu generieren"-Button wird per `addEventListener` verdrahtet, die Optik liegt in neuen `.ki-badge-*`-Klassen in `app.css`. KI-Werte werden außerdem nicht mehr in CSS-Selektoren eingesetzt (Option/Radio werden per Wertvergleich gesucht). +- **Weitere `innerHTML`-Stellen mit Server- bzw. Fremddaten:** `setupAccountSearch()` in `app.js` (Kontobezeichnung), Upload-Status auf Dashboard und Belegliste (Server-Fehlermeldungen), Dateiname im Bankauszug-Upload (`bank/show.php`) und Ollama-Fehlerdetails in den Einstellungen setzen die Werte jetzt als Text. +- **`tests/102_KiBadgeXssTest.php`** (neu) und **`tests/e2e/specs/beleg-ki-badge.spec.js`** (neu): prüfen, dass das Badge keine Serverdaten mehr per `innerHTML` einfügt bzw. dass ein Schadcode-Payload im Browser nur als Text erscheint. + ## 2026-09-20 – Analyse prüft das Importskript (#209) - **`scripts/analyse-nextcloud.sh`:** neuer Abschnitt 6 prüft, ob `scripts/container/nextcloud-import.php` im Container existiert und syntaktisch ladbar ist (`php -l`, ohne Ausführung), und nennt den Pfad, den der Host-Cronjob verwenden muss. Fehlt das Skript, endet die Analyse mit Exit 1 – auch bei erfolgreicher Verbindung. Ein `--dry-run`-Schalter für den Import wurde nicht ergänzt. diff --git a/public/static/css/app.css b/public/static/css/app.css index 688c58f..e627a45 100644 --- a/public/static/css/app.css +++ b/public/static/css/app.css @@ -155,6 +155,14 @@ img { max-width: 100%; } .flash-warning { background: #fef9c3; color: #854d0e; border: 1px solid #fef08a; } .flash-info { background: #dbeafe; color: #1e40af; border: 1px solid #bfdbfe; } +/* KI-Vorschlag-Badge auf der Belegdetailseite (beleg-ki.js) */ +.ki-badge-done { justify-content: space-between; align-items: flex-start; gap: 1rem; } +.ki-badge-details { font-size: .9em; } +.ki-badge-zeile { font-size: .85em; } +.ki-badge-hinweis { color: var(--color-muted); font-style: italic; } +.ki-badge-warn { color: var(--color-warning); font-weight: 600; } +.ki-badge-btn { flex-shrink: 0; margin-top: .1rem; } + /* ── Cards ──────────────────────────────────────────────────── */ .card { background: var(--color-surface); diff --git a/public/static/js/app.js b/public/static/js/app.js index c5ff1cc..840e7d7 100644 --- a/public/static/js/app.js +++ b/public/static/js/app.js @@ -315,7 +315,13 @@ function setupAccountSearch(inputId, hiddenId) { items.forEach(item => { const li = document.createElement('li'); li.style.cssText = 'padding:.5rem .75rem; cursor:pointer; font-size:.875rem; border-bottom:1px solid var(--color-border)'; - li.innerHTML = `${item.kontonummer} – ${item.bezeichnung} (${item.kontenart})`; + // Kontodaten nur als Text einfügen (Fix #213) + const nr = document.createElement('strong'); + nr.textContent = item.kontonummer; + const art = document.createElement('small'); + art.className = 'text-muted'; + art.textContent = `(${item.kontenart})`; + li.append(nr, ` – ${item.bezeichnung} `, art); li.addEventListener('mousedown', () => { input.value = `${item.kontonummer} – ${item.bezeichnung}`; hidden.value = item.id; diff --git a/public/static/js/beleg-ki.js b/public/static/js/beleg-ki.js index 142a974..844010f 100644 --- a/public/static/js/beleg-ki.js +++ b/public/static/js/beleg-ki.js @@ -13,8 +13,19 @@ function _selectById(selectEl, id) { } } +// Option per Wert suchen – ohne den (KI-gelieferten) Wert in einen CSS-Selektor einzusetzen +function _findOption(selectEl, id) { + if (!selectEl) return null; + const str = String(id); + for (const opt of selectEl.options) { + if (opt.value === str) return opt; + } + return null; +} + function _applyKiVorschlag(v) { - const btRadio = document.querySelector('input[name="buchungstyp"][value="' + v.buchungstyp + '"]'); + const btRadio = Array.from(document.querySelectorAll('input[name="buchungstyp"]')) + .find(r => r.value === String(v.buchungstyp)); if (btRadio) { btRadio.checked = true; filterKategorieKonten(v.buchungstyp); } const ustEl = document.getElementById('ust_satz_select'); if (ustEl) { ustEl.value = String(v.ust_satz); updateSteuerartVisibility(); } @@ -49,46 +60,62 @@ function _applyKiVorschlag(v) { } } +// Kleiner Helfer: Element mit Klasse und Textinhalt (niemals HTML) erzeugen +function _kiEl(tag, className, text) { + const el = document.createElement(tag); + if (className) el.className = className; + if (text !== undefined) el.textContent = String(text); + return el; +} + function _showKiBadgeDone(arr) { - // arr ist immer ein Array (bereits normalisiert) + // arr ist immer ein Array (bereits normalisiert). + // Alle Werte stammen aus DB/KI bzw. aus Kontobezeichnungen und werden + // ausschließlich per textContent eingefügt (Fix #213: Stored XSS). const v = arr[0]; - const katEl = document.getElementById('kategorie_konto_id'); - const zhlEl = document.getElementById('zahlungskonto_id'); - const katOpt = katEl ? katEl.querySelector('option[value="' + v.kategorie_konto_id + '"]') : null; - const zhlOpt = zhlEl ? zhlEl.querySelector('option[value="' + v.zahlungskonto_id + '"]') : null; + const katOpt = _findOption(document.getElementById('kategorie_konto_id'), v.kategorie_konto_id); + const zhlOpt = _findOption(document.getElementById('zahlungskonto_id'), v.zahlungskonto_id); const katTxt = katOpt ? katOpt.text : v.kategorie_konto_id; const zhlTxt = zhlOpt ? zhlOpt.text : v.zahlungskonto_id; - const supTxt = v.supplier_name - ? ' · Lieferant: ' + _esc(v.supplier_name) + (v.supplier_found === false ? ' (neu – bitte prüfen)' : '') - : ''; // Erste Zeile (bereits ins Formular übertragen) - let details = + const details = _kiEl('span', 'ki-badge-details', (v.buchungstyp ? v.buchungstyp + ' · ' : '') + 'Kategorie: ' + katTxt + ' · ' + - 'Zahlung: ' + zhlTxt + supTxt; + 'Zahlung: ' + zhlTxt); + if (v.supplier_name) { + details.append(' · Lieferant: ' + v.supplier_name); + if (v.supplier_found === false) { + details.append(' ', _kiEl('span', 'ki-badge-warn', '(neu – bitte prüfen)')); + } + } + + const inhalt = document.createElement('div'); + inhalt.append(_kiEl('strong', '', 'KI-Vorschlag:'), ' ', details); // Alle Buchungszeilen mit Beträgen anzeigen const fmt = n => parseFloat(n).toLocaleString('de-DE', {minimumFractionDigits: 2, maximumFractionDigits: 2}); - let zeilen = ''; arr.forEach((z, i) => { + const satz = parseFloat(z.ust_satz) || 0; const netto = parseFloat(z.betrag_netto) || 0; - const ust = Math.round(netto * z.ust_satz / 100 * 100) / 100; + const ust = Math.round(netto * satz / 100 * 100) / 100; const brutto = Math.round((netto + ust) * 100) / 100; - const hinweis = (i > 0) ? ' (bitte als weitere Buchung erfassen)' : ''; - zeilen += '
Zeile ' + (i + 1) + ': ' - + 'Netto ' + fmt(netto) + ' € | USt ' + z.ust_satz + '% ' + fmt(ust) + ' € | Brutto ' + fmt(brutto) + ' €' - + hinweis + ''; + const zeile = _kiEl('span', 'ki-badge-zeile', + 'Zeile ' + (i + 1) + ': ' + + 'Netto ' + fmt(netto) + ' € | USt ' + satz + '% ' + fmt(ust) + ' € | Brutto ' + fmt(brutto) + ' €'); + if (i > 0) zeile.append(' ', _kiEl('span', 'ki-badge-hinweis', '(bitte als weitere Buchung erfassen)')); + inhalt.append(document.createElement('br'), zeile); }); - const neuBtn = _kiAktiv - ? '' - : ''; - document.getElementById('ki-badge').innerHTML = - '
' + - '
KI-Vorschlag: ' + details + '' + zeilen + '
' + - neuBtn + - '
'; + const box = _kiEl('div', 'flash flash-info ki-badge-done'); + box.append(inhalt); + if (_kiAktiv) { + const neuBtn = _kiEl('button', 'btn btn-secondary btn-sm ki-badge-btn', 'Neu generieren'); + neuBtn.type = 'button'; + neuBtn.addEventListener('click', _startKiAnalyse); + box.append(neuBtn); + } + document.getElementById('ki-badge').replaceChildren(box); } function _showKiBadgeNoResult() { diff --git a/src/Views/bank/show.php b/src/Views/bank/show.php index fbada66..0cfe493 100644 --- a/src/Views/bank/show.php +++ b/src/Views/bank/show.php @@ -184,8 +184,10 @@ $restFormatted = number_format($remaining, 2, ',', '.'); btn.disabled = true; return; } - status.innerHTML = '✓ ' + file.name - + ' (' + (file.size / 1024).toFixed(1) + ' KB)'; + const ok = document.createElement('span'); + ok.style.color = 'var(--color-success)'; + ok.textContent = '✓ ' + file.name + ' (' + (file.size / 1024).toFixed(1) + ' KB)'; + status.replaceChildren(ok); btn.disabled = false; } diff --git a/src/Views/belege/list.php b/src/Views/belege/list.php index 358fa4f..6f4e43f 100644 --- a/src/Views/belege/list.php +++ b/src/Views/belege/list.php @@ -33,7 +33,11 @@ } function setStatus(msg, color) { - status.innerHTML = `${msg}`; + // msg kann Server-Fehlertexte enthalten → nur als Text einfügen (Fix #213) + const span = document.createElement('span'); + span.style.color = color || 'var(--color-muted)'; + span.textContent = msg; + status.replaceChildren(span); } zone.addEventListener('dragover', (e) => { e.preventDefault(); setActive(true); }); diff --git a/src/Views/dashboard.php b/src/Views/dashboard.php index 8e233f7..c49382a 100644 --- a/src/Views/dashboard.php +++ b/src/Views/dashboard.php @@ -105,7 +105,11 @@ $detail = function (string $bucket, ?int $month = null) use ($stats): string { } function setStatus(msg, color) { - status.innerHTML = `${msg}`; + // msg kann Server-Fehlertexte enthalten → nur als Text einfügen (Fix #213) + const span = document.createElement('span'); + span.style.color = color || 'var(--color-muted)'; + span.textContent = msg; + status.replaceChildren(span); } zone.addEventListener('dragover', (e) => { e.preventDefault(); setActive(true); }); diff --git a/src/Views/settings/form.php b/src/Views/settings/form.php index ae6e140..788b305 100644 --- a/src/Views/settings/form.php +++ b/src/Views/settings/form.php @@ -548,7 +548,11 @@ document.getElementById('btn-llm-test')?.addEventListener('click', function () { if (data.elapsed) lines.push('Nach ' + data.elapsed + ' ms'); lines.push('Tipp: sudo ufw allow from 172.16.0.0/12 to any port 11434 (oder firewall prüfen)'); lines.push('Logs: docker compose logs web'); - detail.innerHTML = lines.map(function (l) { return '
' + l + '
'; }).join(''); + detail.replaceChildren.apply(detail, lines.map(function (l) { + var d = document.createElement('div'); + d.textContent = l; + return d; + })); detail.style.display = 'block'; detail.style.color = '#dc2626'; } diff --git a/tests/102_KiBadgeXssTest.php b/tests/102_KiBadgeXssTest.php new file mode 100644 index 0000000..0bbaa72 --- /dev/null +++ b/tests/102_KiBadgeXssTest.php @@ -0,0 +1,75 @@ +functionBody($js, '_showKiBadgeDone'); + $this->assertNotEmpty($body, '_showKiBadgeDone() in beleg-ki.js gefunden'); + $this->assert(!str_contains($body, 'innerHTML'), '_showKiBadgeDone() setzt kein innerHTML'); + $this->assert(!str_contains($body, 'onclick'), '_showKiBadgeDone() erzeugt keinen Inline-Handler'); + $this->assert(str_contains($body, 'textContent'), '_showKiBadgeDone() fügt Werte per textContent ein'); + $this->assert( + !preg_match('/querySelector\([^)]*\+\s*v\./', $js), + 'KI-Werte werden nicht in CSS-Selektoren eingesetzt' + ); + + $app = (string)file_get_contents($root . '/public/static/js/app.js'); + $this->assert( + !str_contains($app, '${item.bezeichnung} assert(!str_contains($content, '}">${msg}'), "$view: Statusmeldung nicht per innerHTML"); + } + + $settings = (string)file_get_contents($root . '/src/Views/settings/form.php'); + $this->assert( + !str_contains($settings, "'
' + l + '
'"), + 'settings/form.php: Ollama-Fehlerdetails nicht per innerHTML' + ); + } + + /** Liefert den Rumpf einer JS-Funktion (bis zur passenden schließenden Klammer). */ + private function functionBody(string $js, string $name): string + { + $pos = strpos($js, 'function ' . $name . '('); + if ($pos === false) { + return ''; + } + $start = strpos($js, '{', $pos); + if ($start === false) { + return ''; + } + $depth = 0; + $len = strlen($js); + for ($i = $start; $i < $len; $i++) { + if ($js[$i] === '{') { + $depth++; + } elseif ($js[$i] === '}') { + $depth--; + if ($depth === 0) { + return substr($js, $start, $i - $start + 1); + } + } + } + return ''; + } +} diff --git a/tests/e2e/specs/beleg-ki-badge.spec.js b/tests/e2e/specs/beleg-ki-badge.spec.js new file mode 100644 index 0000000..23f7c09 --- /dev/null +++ b/tests/e2e/specs/beleg-ki-badge.spec.js @@ -0,0 +1,74 @@ +// @ts-check +// Issue #213: Stored XSS über Kontobezeichnungen im KI-Vorschlag-Badge. +// +// Das Badge liest die Kontobezeichnung aus den