From 30378683e65ced2c1b47b0fd63c66a2508bbabdc Mon Sep 17 00:00:00 2001 From: Clemens Creutzburg Date: Sun, 12 Jul 2026 10:13:56 +0200 Subject: [PATCH] =?UTF-8?q?CSV-Upload=20gegen=20CSRF=20und=20unsichere=20A?= =?UTF-8?q?blage=20h=C3=A4rten?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Upload-Formular mit CSRF-Token absichern - CSV-Dateien temporär unter var/uploads speichern und nach Import löschen - Dateityp, Dateigröße und Dateiendung prüfen - CSV-Ausgabe escapen und PayPal-Name-Lookup korrigieren - M2- und Sicherheitsdokumentation aktualisieren --- csvupload.php | 142 +++++++++++++++++++++++------ docs/m0/security-baseline.md | 8 +- docs/m2-technical-foundation.md | 29 +++++- docs/saas-umstrukturierungsplan.md | 5 +- 4 files changed, 144 insertions(+), 40 deletions(-) diff --git a/csvupload.php b/csvupload.php index ea0c001..a17be34 100644 --- a/csvupload.php +++ b/csvupload.php @@ -1,6 +1,7 @@ 5 * 1024 * 1024) { + return [null, 'Die CSV-Datei ist leer oder groesser als 5 MB.']; + } + + $originalName = (string)($file['name'] ?? ''); + $extension = strtolower(pathinfo($originalName, PATHINFO_EXTENSION)); + if ($extension !== 'csv') { + return [null, 'Es sind nur CSV-Dateien erlaubt.']; + } + + $tmpName = (string)($file['tmp_name'] ?? ''); + if (!is_uploaded_file($tmpName)) { + return [null, 'Die hochgeladene Datei konnte nicht verifiziert werden.']; + } + + if (function_exists('finfo_open')) { + $finfo = finfo_open(FILEINFO_MIME_TYPE); + $mime = $finfo ? finfo_file($finfo, $tmpName) : false; + if ($finfo) { + finfo_close($finfo); + } + + $allowedMimeTypes = [ + 'text/plain', + 'text/csv', + 'text/x-csv', + 'application/csv', + 'application/vnd.ms-excel', + 'application/octet-stream', + ]; + if (is_string($mime) && !in_array($mime, $allowedMimeTypes, true)) { + return [null, 'Der Dateityp wurde nicht als CSV erkannt.']; + } + } + + $uploadDir = csv_upload_dir(); + if (!is_dir($uploadDir)) { + @mkdir($uploadDir, 0700, true); + } + if (!is_dir($uploadDir) || !is_writable($uploadDir)) { + return [null, 'Der Upload-Ordner ist nicht beschreibbar.']; + } + + $targetFile = $uploadDir . '/paypal_' . date('Ymd_His') . '_' . bin2hex(random_bytes(8)) . '.csv'; + if (!move_uploaded_file($tmpName, $targetFile)) { + return [null, 'Die CSV-Datei konnte nicht gespeichert werden.']; + } + + return [$targetFile, null]; +} + // Funktion zum Überprüfen, ob der Mitarbeiter in der Tabelle vorhanden ist function isMitarbeiterExist($conn, $name) { - $sql = "SELECT MitarbeiterID FROM kl_Mitarbeiter WHERE Name = ?"; - $params = array($name); + $sql = "SELECT MitarbeiterID FROM kl_Mitarbeiter WHERE Name = ? or paypalname = ?"; + $params = array($name, $name); $stmt = sqlsrv_query($conn, $sql, $params); if ($stmt === false) { @@ -36,7 +104,12 @@ function isMitarbeiterExist($conn, $name) function isDuplicateEntry($conn, $mitarbeiterID, $betrag, $datum) { // Um nur den gleichen Tag zu vergleichen, konvertieren wir den Datum-String zu einem DateTime-Objekt und verwenden die Funktion CONVERT - $convertedDatum = date_format(date_create($datum), 'Y-m-d'); + $date = date_create($datum); + if ($date === false) { + return false; + } + + $convertedDatum = date_format($date, 'Y-m-d'); #$sql = "SELECT count FROM dbo.kl_Einzahlungen WHERE MitarbeiterID = ? AND Betrag = ? AND CONVERT(VARCHAR, Datum, 23) = ?"; $sql = "SELECT * FROM dbo.kl_Einzahlungen WHERE MitarbeiterID = ? AND Betrag = ? AND CONVERT(VARCHAR, Datum, 23) = ?"; @@ -49,7 +122,7 @@ function isDuplicateEntry($conn, $mitarbeiterID, $betrag, $datum) $row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC); - return ($row['MitarbeiterID'] > 0); + return $row !== null && (int)$row['MitarbeiterID'] > 0; } // Funktion zum Verarbeiten der CSV-Datei und Rückgabe der Ergebnisse @@ -63,6 +136,11 @@ function processCSV($conn, $file) fgetcsv($handle); while (($data = fgetcsv($handle, 1000, ",")) !== FALSE) { + if (count($data) < 8) { + $failed_entries[] = array($data[0] ?? '', $data[3] ?? '', $data[7] ?? '', 4); + continue; + } + $name = $data[3]; // Index 3 entspricht dem "Name"-Feld in der CSV $betrag = $data[7]; // Index 7 entspricht dem "Brutto"-Feld in der CSV $betrag = str_replace(",", ".", $betrag); @@ -108,7 +186,7 @@ function processCSV($conn, $file) function getMitarbeiterID($conn, $name) { $sql = "SELECT MitarbeiterID FROM kl_Mitarbeiter WHERE Name = ? or paypalname = ?"; - $params = array($name); + $params = array($name, $name); $stmt = sqlsrv_query($conn, $sql, $params); @@ -127,20 +205,20 @@ function getMitarbeiterID($conn, $name) // Überprüfen, ob das Formular abgeschickt wurde if ($_SERVER["REQUEST_METHOD"] == "POST") { // Überprüfen, ob eine Datei hochgeladen wurde - if (isset($_FILES["csv_file"]) && $_FILES["csv_file"]["error"] == 0) { - $file_name = $_FILES["csv_file"]["name"]; - $file_tmp = $_FILES["csv_file"]["tmp_name"]; + if (isset($_FILES["csv_file"])) { + [$csvFile, $uploadError] = csv_prepare_upload($_FILES["csv_file"]); - // Datei in den Upload-Ordner verschieben - move_uploaded_file($file_tmp, "uploads/" . $file_name); + if ($uploadError !== null) { + echo csv_h($uploadError); + } else { + // CSV-Datei verarbeiten + $result = processCSV($conn, $csvFile); + @unlink($csvFile); - // CSV-Datei verarbeiten - $result = processCSV($conn, "uploads/" . $file_name); + echo "CSV-Datei erfolgreich verarbeitet.\n"; - echo "CSV-Datei erfolgreich verarbeitet.\n"; - - echo "

Auswertung

"; - echo " + echo "

Auswertung

"; + echo "
@@ -149,19 +227,19 @@ if ($_SERVER["REQUEST_METHOD"] == "POST") { "; - foreach ($result['success'] as $eintrag){ - echo " - - - + foreach ($result['success'] as $eintrag){ + echo " + + + "; - } - foreach ($result['failed'] as $eintrag){ - echo " - - - + } + foreach ($result['failed'] as $eintrag){ + echo " + + + "; - } - echo "
NameErgebnis
$eintrag[0]$eintrag[1]$eintrag[2]
" . csv_h($eintrag[0]) . "" . csv_h($eintrag[1]) . "" . csv_h($eintrag[2]) . " Erfolgreich gespeichert
$eintrag[0]$eintrag[1]$eintrag[2]
" . csv_h($eintrag[0]) . "" . csv_h($eintrag[1]) . "" . csv_h($eintrag[2]) . " "; if($eintrag[3] == 1){ echo "SQL Fehler"; @@ -169,12 +247,15 @@ if ($_SERVER["REQUEST_METHOD"] == "POST") { echo "Eintrag schon vorhanden"; }elseif($eintrag[3] == 3){ echo "Benutzer nicht gefunden"; + }elseif($eintrag[3] == 4){ + echo "Ungueltige CSV-Zeile"; } echo "
"; + } + echo ""; + } } else { echo "Fehler beim Hochladen der Datei."; @@ -193,6 +274,7 @@ if ($_SERVER["REQUEST_METHOD"] == "POST") {
" method="post" enctype="multipart/form-data"> + @@ -215,4 +297,4 @@ if ($_SERVER["REQUEST_METHOD"] == "POST") { - \ No newline at end of file + diff --git a/docs/m0/security-baseline.md b/docs/m0/security-baseline.md index a8a5be2..8b4e810 100644 --- a/docs/m0/security-baseline.md +++ b/docs/m0/security-baseline.md @@ -11,9 +11,9 @@ Testing, markiert aber die wichtigsten Risiken fuer die SaaS-Umstrukturierung. | --- | --- | --- | --- | | Hart codierte DB-Zugangsdaten | `jahresauswertung.php` Zeilen 4-8 | Secret-Leak, direkte Produktiv-DB-Gefahr | Zugangsdaten rotieren, Skript deaktivieren oder auf Env-Konfiguration umstellen | | Schreibende Seiten ohne eigene Rollenpruefung | `stricheintragen.php`, `einzahlung.php`, `mailversenden.php`, `exportKaffeeliste.php` | Direkter URL-Aufruf kann Aktionen erlauben | Jede Seite serverseitig mit `requireRole` absichern | -| CSRF-Schutz nur teilweise vorhanden | viele POST-/Delete-Formulare; M2 hat `hinweise.php`, `mitarbeiterverwalten.php`, `namenanpassen.php`, `index.php`, `stricheintragen.php`, `einzahlung.php`, `letzteneintraege.php` abgesichert | Ungewollte Buchungen, Loeschungen, Imports | CSRF fuer alle verbleibenden schreibenden Aktionen | +| CSRF-Schutz nur teilweise vorhanden | viele POST-/Delete-Formulare; M2 hat `hinweise.php`, `mitarbeiterverwalten.php`, `namenanpassen.php`, `index.php`, `stricheintragen.php`, `einzahlung.php`, `letzteneintraege.php`, `csvupload.php` abgesichert | Ungewollte Buchungen, Loeschungen, Imports | CSRF fuer alle verbleibenden schreibenden Aktionen | | Harte Deletes fuer Buchungen | `letzteneintraege.php` | Audit-Historie und Revisionsfaehigkeit gehen verloren | Storno-/Reversal-Modell statt Delete | -| Upload in Webroot | `csvupload.php` Zeilen 131-138 | Dateiablage kann missbraucht werden | Upload ausserhalb Webroot, Dateityp/MIME/Name pruefen | +| CSV-Upload nur teilweise gehaertet | `csvupload.php`; M2 speichert temporaer unter `var/uploads`, prueft Dateityp und loescht nach Import | Ohne Importvorschau/Audit bleiben Fehlimporte schwer nachvollziehbar | Importvorschau, Batch-/Audit-Log und detaillierte Zeilenfehler in M6 | | Kein Tenant-Scope | alle fachlichen Queries | Zentrales SaaS-Leak-Risiko | `tenant_id` verpflichtend und Query-Schicht testen | ## Weitere Befunde @@ -25,7 +25,7 @@ Testing, markiert aber die wichtigsten Risiken fuer die SaaS-Umstrukturierung. | Datumsformat `Y-d-m H:i:s` | `index.php`, `stricheintragen.php`, `einzahlung.php`, `hinweise.php` | ISO/DB-kompatibel `Y-m-d H:i:s` oder DB-Zeit verwenden | | Ausgabe von Namen/E-Mails teils unescaped | mehrere Tabellen, z.B. Mitgliederverwaltung | `htmlspecialchars` zentral erzwingen | | Fehlerausgabe mit `sqlsrv_errors()` an Nutzer | mehrere Dateien | Logging intern, neutrale Fehlermeldung extern | -| CSV-Mitarbeitersuche mit mutmasslich falscher Parameteranzahl | `csvupload.php` `getMitarbeiterID` | Query pruefen und mit Tests abdecken | +| CSV-Mitarbeitersuche mit mutmasslich falscher Parameteranzahl | `csvupload.php` `getMitarbeiterID`; in M2 korrigiert | Mit Golden-Master weiter pruefen | | Basis-Auth-Beispiel mit Platzhalter-Passwort | `umfrageergebnisse.php` Kommentarblock | Entfernen oder echte Auth-Middleware nutzen | | App-Navigation ist in `footer.php` | Layoutstruktur | Trennung in App-Shell und Public-Shell | | `headerline.php` enthaelt NUL-Zeichen | `headerline.php` | Datei pruefen/entfernen, wenn ungenutzt | @@ -54,7 +54,7 @@ Fuer SaaS gilt: 2. Legacy-Schreibseiten bis zum Umbau hinter explizite Admin-Pruefung setzen. 3. CSRF-Schutz schrittweise auf alle verbleibenden Legacy-Schreibseiten ausrollen. 4. Finanzdaten im Zielmodell nur noch stornieren, nicht loeschen. -5. Uploads ausserhalb des Webroots modellieren. +5. CSV-Import mit Vorschau, Audit und Zeilenfehlern modellieren. 6. Einheitliche Escape-/View-Helfer einfuehren. 7. Tenant-Isolation mit Tests gegen zwei Tenants absichern. diff --git a/docs/m2-technical-foundation.md b/docs/m2-technical-foundation.md index 0b50919..8167950 100644 --- a/docs/m2-technical-foundation.md +++ b/docs/m2-technical-foundation.md @@ -48,12 +48,29 @@ Erste Legacy-POST-Seiten sind opt-in abgesichert: - `stricheintragen.php`: Sammelerfassung von Strichen. - `einzahlung.php`: Sammelerfassung von Einzahlungen. - `letzteneintraege.php`: letzte Einzahlungen und Strich-Eintraege loeschen. +- `csvupload.php`: CSV-Zahlungsimport. Noch offen: -- Uploads: `csvupload.php`. - Spezialprozesse: `mailversenden.php`, `jahresauswertung.php`. +### CSV-Upload-Haertung + +- `csvupload.php` nutzt jetzt CSRF. +- Uploads werden unter `var/uploads` gespeichert und nach der Verarbeitung + geloescht. Damit liegen importierte Dateien nicht mehr im Webroot. +- Dateiendung, Dateigroesse und MIME-Typ werden vor der Verarbeitung geprueft. +- Hochgeladene Dateien bekommen serverseitig erzeugte Zufallsnamen. +- CSV-Auswertungswerte werden HTML-escaped ausgegeben. +- Die PayPal-Namenssuche nutzt `Name` und `paypalname` mit expliziter + Parameterbindung. + +Offen fuer M6: + +- Importvorschau vor dem Schreiben. +- Import-Batch/Audit-Log. +- Saubere Fehlerberichte je CSV-Zeile. + ### Migrationen - `database/migrations/0001_legacy_mysql_baseline.sql` bildet die bisherige @@ -115,6 +132,10 @@ Die Skripte erwarten die bekannten Dev-Umgebungsvariablen `DB_HOST`, `DB_NAME`, liefert bei POST ohne Token HTTP 419. - CSRF positive Tests fuer Korrektur-/Loeschflows: gueltige Token funktionieren fuer das Loeschen temporaerer Einzahlungs- und Strich-Testeintraege. +- CSRF negative Test fuer CSV-Upload: POST ohne Token liefert HTTP 419. +- CSV-Upload positive Tests: gueltiges Token verarbeitet eine CSV-Datei, erkennt + eine PayPal-Alias-Dublette und hinterlaesst keine Datei in `var/uploads`. +- CSV-Upload negative Tests: Nicht-CSV-Dateien werden abgewiesen. - Golden Master weiterhin gruen mit 104 Assertions. - HTTP-Smoke weiterhin gruen mit 14 sicheren Seiten. @@ -128,8 +149,6 @@ Die Skripte erwarten die bekannten Dev-Umgebungsvariablen `DB_HOST`, `DB_NAME`, ## Naechste Schritte -1. CSV-Upload gesondert absichern und spaeter Uploads ausserhalb des Webroots - verlegen. -2. Eine duenne View-/Layout-Struktur vorbereiten, ohne Header/Footer-Markup +1. Eine duenne View-/Layout-Struktur vorbereiten, ohne Header/Footer-Markup sofort zu verschieben. -3. Danach M3 starten: Tenants, User, Registrierung, Login und Rollen. +2. Danach M3 starten: Tenants, User, Registrierung, Login und Rollen. diff --git a/docs/saas-umstrukturierungsplan.md b/docs/saas-umstrukturierungsplan.md index 16a3cc3..ce7fed9 100644 --- a/docs/saas-umstrukturierungsplan.md +++ b/docs/saas-umstrukturierungsplan.md @@ -384,7 +384,10 @@ Schritte: globale Erzwingung erfolgt schrittweise pro POST-Seite. Erste Seiten sind abgesichert: `hinweise.php`, `mitarbeiterverwalten.php`, `namenanpassen.php`, `index.php`, `stricheintragen.php`, `einzahlung.php`, - `letzteneintraege.php`. + `letzteneintraege.php`, `csvupload.php`. +- CSV-Upload ausserhalb des Webroots speichern. In M2 als Legacy-Haertung + umgesetzt: temporaer unter `var/uploads`, Dateityp-/Groessenpruefung und + Loeschung nach Verarbeitung. - Konfigurationswerte aus Code in Umgebung oder Settings verschieben. Ergebnis: