Merge pull request 'Fix #213: Stored XSS über Kontobezeichnung im KI-Vorschlag-Badge' (#214) from fds2610-bot/Steuerkram:main into main
Reviewed-on: https://codeberg.org/fds2610/Steuerkram/pulls/214
This commit is contained in:
fds2610
commit
870f0f2118
10 changed files with 241 additions and 31 deletions
|
|
@ -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<img src=x onerror=…>` 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.
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 = `<strong>${item.kontonummer}</strong> – ${item.bezeichnung} <small style="color:var(--color-muted)">(${item.kontenart})</small>`;
|
||||
// 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;
|
||||
|
|
|
|||
|
|
@ -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 ? ' <span style="color:var(--color-warning);font-weight:600">(neu – bitte prüfen)</span>' : '')
|
||||
: '';
|
||||
|
||||
// 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) ? ' <span style="color:var(--color-muted);font-style:italic">(bitte als weitere Buchung erfassen)</span>' : '';
|
||||
zeilen += '<br><span style="font-size:.85em">Zeile ' + (i + 1) + ': '
|
||||
+ 'Netto ' + fmt(netto) + ' € | USt ' + z.ust_satz + '% ' + fmt(ust) + ' € | Brutto ' + fmt(brutto) + ' €'
|
||||
+ hinweis + '</span>';
|
||||
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
|
||||
? '<button class="btn btn-secondary btn-sm" style="flex-shrink:0;margin-top:.1rem" onclick="_startKiAnalyse()">Neu generieren</button>'
|
||||
: '';
|
||||
document.getElementById('ki-badge').innerHTML =
|
||||
'<div class="flash flash-info" style="display:flex;justify-content:space-between;align-items:flex-start;gap:1rem">' +
|
||||
'<div><strong>KI-Vorschlag:</strong> <span style="font-size:.9em">' + details + '</span>' + zeilen + '</div>' +
|
||||
neuBtn +
|
||||
'</div>';
|
||||
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() {
|
||||
|
|
|
|||
|
|
@ -184,8 +184,10 @@ $restFormatted = number_format($remaining, 2, ',', '.');
|
|||
btn.disabled = true;
|
||||
return;
|
||||
}
|
||||
status.innerHTML = '<span style="color:var(--color-success)">✓ ' + file.name
|
||||
+ ' (' + (file.size / 1024).toFixed(1) + ' KB)</span>';
|
||||
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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -33,7 +33,11 @@
|
|||
}
|
||||
|
||||
function setStatus(msg, color) {
|
||||
status.innerHTML = `<span style="color:${color || 'var(--color-muted)'}">${msg}</span>`;
|
||||
// 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); });
|
||||
|
|
|
|||
|
|
@ -105,7 +105,11 @@ $detail = function (string $bucket, ?int $month = null) use ($stats): string {
|
|||
}
|
||||
|
||||
function setStatus(msg, color) {
|
||||
status.innerHTML = `<span style="color:${color || 'var(--color-muted)'}">${msg}</span>`;
|
||||
// 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); });
|
||||
|
|
|
|||
|
|
@ -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 '<div>' + l + '</div>'; }).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';
|
||||
}
|
||||
|
|
|
|||
75
tests/102_KiBadgeXssTest.php
Normal file
75
tests/102_KiBadgeXssTest.php
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* Test 102: Kein Stored XSS über Kontobezeichnungen im KI-Vorschlag-Badge (Issue #213)
|
||||
*
|
||||
* Das Badge wird im Browser aus Kontobezeichnungen (option.text, bereits
|
||||
* dekodiert) und KI-Daten gebaut. Der Test stellt sicher, dass diese Werte
|
||||
* nicht mehr per innerHTML eingefügt werden. Das Verhalten im Browser prüft
|
||||
* tests/e2e/specs/beleg-ki-badge.spec.js.
|
||||
*/
|
||||
class KiBadgeXssTest extends TestCase
|
||||
{
|
||||
protected string $testName = 'Test 102: KI-Badge ohne innerHTML für Serverdaten';
|
||||
|
||||
public function run(): void
|
||||
{
|
||||
$root = __DIR__ . '/..';
|
||||
$js = (string)file_get_contents($root . '/public/static/js/beleg-ki.js');
|
||||
|
||||
$body = $this->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} <small'),
|
||||
'setupAccountSearch() fügt die Kontobezeichnung nicht per innerHTML ein'
|
||||
);
|
||||
|
||||
foreach (['src/Views/dashboard.php', 'src/Views/belege/list.php'] as $view) {
|
||||
$content = (string)file_get_contents($root . '/' . $view);
|
||||
$this->assert(!str_contains($content, '}">${msg}</span>'), "$view: Statusmeldung nicht per innerHTML");
|
||||
}
|
||||
|
||||
$settings = (string)file_get_contents($root . '/src/Views/settings/form.php');
|
||||
$this->assert(
|
||||
!str_contains($settings, "'<div>' + l + '</div>'"),
|
||||
'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 '';
|
||||
}
|
||||
}
|
||||
74
tests/e2e/specs/beleg-ki-badge.spec.js
Normal file
74
tests/e2e/specs/beleg-ki-badge.spec.js
Normal file
|
|
@ -0,0 +1,74 @@
|
|||
// @ts-check
|
||||
// Issue #213: Stored XSS über Kontobezeichnungen im KI-Vorschlag-Badge.
|
||||
//
|
||||
// Das Badge liest die Kontobezeichnung aus den <option>-Elementen (option.text
|
||||
// ist bereits dekodiert) und baut daraus die Anzeige. Einen gespeicherten
|
||||
// KI-Vorschlag kann der Test ohne Ollama nicht erzeugen, und bestehende Konten
|
||||
// darf er nicht umbenennen. Er legt deshalb im Browser eine Option mit
|
||||
// Schadcode-Bezeichnung an und ruft _showKiBadgeDone() direkt auf.
|
||||
|
||||
const { test, expect } = require('@playwright/test')
|
||||
const { uploadBeleg, cleanupBeleg } = require('../helpers/app')
|
||||
|
||||
const PAYLOAD = '<img src=x onerror="window.__xss213=1">'
|
||||
|
||||
test.describe('KI-Vorschlag-Badge', () => {
|
||||
let belegId = 0
|
||||
|
||||
test.afterEach(async ({ page }) => {
|
||||
await cleanupBeleg(page, belegId)
|
||||
belegId = 0
|
||||
})
|
||||
|
||||
test('Kontobezeichnungen und KI-Werte werden nur als Text angezeigt', async ({ page }) => {
|
||||
const { id } = await uploadBeleg(page)
|
||||
belegId = id
|
||||
|
||||
// Ist die KI-Sektion in dieser Instanz aus, fehlen Badge und Skript:
|
||||
// dann beides nachladen (ohne Vorschlag, ohne automatische Analyse).
|
||||
if (await page.locator('#ki-badge').count() === 0) {
|
||||
await page.evaluate(() => {
|
||||
const el = document.createElement('div')
|
||||
el.id = 'ki-badge'
|
||||
el.dataset.belegId = '0'
|
||||
el.dataset.kiAktiv = 'false'
|
||||
el.dataset.kiVorschlag = '[]'
|
||||
document.querySelector('main, body').appendChild(el)
|
||||
})
|
||||
await page.addScriptTag({ url: '/static/js/beleg-ki.js' })
|
||||
}
|
||||
|
||||
await page.evaluate(payload => {
|
||||
for (const [selId, value] of [['kategorie_konto_id', '999001'], ['zahlungskonto_id', '999002']]) {
|
||||
const opt = document.createElement('option')
|
||||
opt.value = value
|
||||
opt.textContent = 'Bank' + payload
|
||||
document.getElementById(selId).appendChild(opt)
|
||||
}
|
||||
// eslint-disable-next-line no-undef
|
||||
_showKiBadgeDone([{
|
||||
buchungstyp: 'Ausgabe' + payload,
|
||||
kategorie_konto_id: '999001',
|
||||
zahlungskonto_id: '999002',
|
||||
supplier_name: 'Lieferant' + payload,
|
||||
supplier_found: false,
|
||||
betrag_netto: '100',
|
||||
ust_satz: '19' + payload,
|
||||
}, { betrag_netto: '10', ust_satz: 7 }])
|
||||
}, PAYLOAD)
|
||||
|
||||
const badge = page.locator('#ki-badge')
|
||||
await expect(badge).toContainText('KI-Vorschlag:')
|
||||
await expect(badge).toContainText('Kategorie: Bank' + PAYLOAD)
|
||||
await expect(badge).toContainText('Zahlung: Bank' + PAYLOAD)
|
||||
await expect(badge).toContainText('Lieferant: Lieferant' + PAYLOAD)
|
||||
await expect(badge).toContainText('(neu – bitte prüfen)')
|
||||
await expect(badge).toContainText('Zeile 1: Netto 100,00 € | USt 19% 19,00 € | Brutto 119,00 €')
|
||||
await expect(badge).toContainText('Zeile 2: Netto 10,00 € | USt 7% 0,70 € | Brutto 10,70 €')
|
||||
await expect(badge).toContainText('(bitte als weitere Buchung erfassen)')
|
||||
await expect(badge.locator('img')).toHaveCount(0)
|
||||
|
||||
await page.waitForTimeout(300)
|
||||
expect(await page.evaluate(() => /** @type {any} */ (window).__xss213)).toBeUndefined()
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Reference in a new issue