diff --git a/Changes.md b/Changes.md index 28e6c30..74f4cc1 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,16 @@ --- +## 2026-07-08 – Fix: Unauthentifizierter Direktzugriff auf Belege/Rechnungs-PDFs (Issue #116) + +- **Kritisch:** Hochgeladene Belege und Rechnungs-PDFs lagen unter `public/static/uploads/...` im Web-Root und wurden von Apache direkt ausgeliefert, bevor der Request `index.php`/`Auth::require()` erreichte – ein Angreifer konnte Belege ohne Anmeldung per URL abrufen. +- **`src/Config.php`:** `upload_dir` zeigt jetzt auf `data/uploads` (außerhalb des Web-Roots, analog zum bereits per Apache gesperrten `/data`-Verzeichnis) +- **`src/Models/Beleg.php`:** Neue Methode `absolutePath()` als einzige Quelle für die Pfad-Auflösung; unterstützt weiterhin Bestandsdaten mit dem alten Präfix `static/uploads/` in `storage_path` +- **`src/Controllers/BelegController.php`, `InvoiceController.php`, `BankController.php`, `src/Services/NextcloudImportService.php`, `KontierungsvorschlagService.php`:** Alle Stellen, die Beleg-Dateien schreiben oder lesen, nutzen jetzt `Config::get('upload_dir')` bzw. `Beleg::absolutePath()` statt eigener `public/static/uploads`-Pfade +- **`docker-compose.yml`, `Dockerfile`, `docker-entrypoint.sh`:** Upload-Verzeichnis wird nach `/var/www/html/data/uploads` gemountet/angelegt (bestehender Host-Ordner `./uploads` bleibt unverändert) + +--- + ## 2026-07-06 – Fix: Fehlermeldung beim Beleg-Löschen, falls Datei nicht löschbar ist (Issue #114) - **`src/Models/Beleg.php`:** `delete()` nutzte ein ungeschütztes `unlink()`; schlug dieses auf dem Server mit „Permission denied" fehl, wurde eine PHP-Warning ausgegeben, die den nachfolgenden Redirect-Header zerstörte („headers already sent"). Der Datensatz wurde dabei trotzdem gelöscht, was zu Verwirrung führte. Fehler beim Datei-Löschen wird jetzt abgefangen und nur noch geloggt (`error_log`), die Löschung des Datensatzes läuft unverändert durch. diff --git a/Dockerfile b/Dockerfile index f6f169d..0e049c6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -36,9 +36,8 @@ COPY apache/steuerkram.conf /etc/apache2/sites-available/000-default.conf RUN a2enmod rewrite headers # Permissions -RUN mkdir -p /var/www/html/public/static/uploads /var/www/html/data \ +RUN mkdir -p /var/www/html/data/uploads \ && chown -R www-data:www-data /var/www/html \ - && chmod -R 755 /var/www/html/public/static/uploads \ && chmod -R 755 /var/www/html/data EXPOSE 80 diff --git a/docker-compose.yml b/docker-compose.yml index 848f2bf..2acba01 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,7 +7,7 @@ services: - ./public:/var/www/html/public - ./src:/var/www/html/src - ./migrations:/var/www/html/migrations - - ./uploads:/var/www/html/public/static/uploads + - ./uploads:/var/www/html/data/uploads - ./data:/var/www/html/data - ./tests:/var/www/html/tests - ./scripts:/var/www/html/scripts diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 360f83a..aed7b43 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -1,6 +1,6 @@ #!/bin/sh # Fix ownership of volume-mounted upload directory at container start -chown -R www-data:www-data /var/www/html/public/static/uploads 2>/dev/null || true +chown -R www-data:www-data /var/www/html/data/uploads 2>/dev/null || true # Datenbank-Migrationen automatisch einspielen php /var/www/html/scripts/migrate.php diff --git a/docs/datenbankstruktur.md b/docs/datenbankstruktur.md index b823d68..e80bfb9 100644 --- a/docs/datenbankstruktur.md +++ b/docs/datenbankstruktur.md @@ -116,7 +116,7 @@ MariaDB 11, Engine InnoDB, Zeichensatz `utf8mb4_unicode_ci`. Das Schema wird dur | `id` | INT UNSIGNED PK | | | `beleg_nr` | VARCHAR(30) UNIQUE | Automatisch vergeben (z.B. `BLG-2026-0001`) | | `filename` | VARCHAR(500) | Originaldateiname | -| `storage_path` | VARCHAR(500) | Pfad unter `public/static/uploads/` | +| `storage_path` | VARCHAR(500) | Relativer Pfad unter `data/uploads/` (außerhalb des Web-Roots, Bestandsdaten mit altem Präfix `static/uploads/` werden von `Beleg::absolutePath()` weiter unterstützt) | | `mime_type` | VARCHAR(100) | MIME-Typ der Datei | | `file_hash` | VARCHAR(64) | SHA-256 für GoBD-Integritätsprüfung | | `beleg_datum` | DATE | Datum auf dem Dokument | diff --git a/src/Config.php b/src/Config.php index 5b36f7e..c43a6c6 100644 --- a/src/Config.php +++ b/src/Config.php @@ -17,7 +17,7 @@ class Config 'db_pass' => $_ENV['DB_PASS'] ?? getenv('DB_PASS') ?: 'steuerkram', 'app_key' => $_ENV['APP_KEY'] ?? getenv('APP_KEY') ?: '', 'sidebar_bg_color' => $_ENV['SIDEBAR_BG_COLOR'] ?? getenv('SIDEBAR_BG_COLOR') ?: '#1e293b', - 'upload_dir' => dirname(__DIR__) . '/public/static/uploads', + 'upload_dir' => dirname(__DIR__) . '/data/uploads', 'base_url' => '', ]; } diff --git a/src/Controllers/BankController.php b/src/Controllers/BankController.php index 37b65a5..7f962ef 100644 --- a/src/Controllers/BankController.php +++ b/src/Controllers/BankController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Config; use Steuerkram\Models\BankProvider; use Steuerkram\Models\BankSource; use Steuerkram\Models\BankTransaction; @@ -350,12 +351,12 @@ class BankController extends BaseController $ext = $allowedMime[$mime]; $belegNr = Beleg::generateBelegNr(); $year = date('Y', strtotime($transaction['wertstellung'])); - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir)) mkdir($absDir, 0755, true); $storagePath = "{$relDir}/original.{$ext}"; - $absPath = dirname(__DIR__, 2) . "/public/{$storagePath}"; + $absPath = Config::get('upload_dir') . "/{$storagePath}"; move_uploaded_file($file['tmp_name'], $absPath); $hash = hash_file('sha256', $absPath); diff --git a/src/Controllers/BelegController.php b/src/Controllers/BelegController.php index eb98f58..23b8ad8 100644 --- a/src/Controllers/BelegController.php +++ b/src/Controllers/BelegController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Config; use Steuerkram\Models\Beleg; use Steuerkram\Models\Booking; use Steuerkram\Models\Account; @@ -159,14 +160,14 @@ class BelegController extends BaseController : date('Y'); // Store under uploads/{year}/{belegNr}/original.{ext} - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir)) mkdir($absDir, 0755, true); $filename = pathinfo($file['name'], PATHINFO_BASENAME); $filenameWithoutExt = pathinfo($file['name'], PATHINFO_FILENAME); $storagePath = "{$relDir}/original.{$ext}"; - $absPath = dirname(__DIR__, 2) . "/public/{$storagePath}"; + $absPath = Config::get('upload_dir') . "/{$storagePath}"; move_uploaded_file($file['tmp_name'], $absPath); @@ -311,8 +312,8 @@ class BelegController extends BaseController exit; } - $basePath = (string)realpath(dirname(__DIR__, 2) . '/public/static/uploads'); - $absPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $basePath = (string)realpath(Config::get('upload_dir')); + $absPath = Beleg::absolutePath($beleg); $realPath = realpath($absPath); if ($realPath === false || !str_starts_with($realPath, $basePath)) { @@ -466,7 +467,7 @@ class BelegController extends BaseController return; } - $absPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $absPath = Beleg::absolutePath($beleg); if (!file_exists($absPath)) { http_response_code(404); echo json_encode(['error' => 'Datei nicht gefunden.']); diff --git a/src/Controllers/InvoiceController.php b/src/Controllers/InvoiceController.php index 72d9384..b5187fe 100644 --- a/src/Controllers/InvoiceController.php +++ b/src/Controllers/InvoiceController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Config; use Steuerkram\Models\Invoice; use Steuerkram\Models\Customer; use Steuerkram\Models\Beleg; @@ -291,8 +292,8 @@ class InvoiceController extends BaseController $beleg = null; if (!empty($invoice['beleg_id'])) { $beleg = Beleg::find((int)$invoice['beleg_id']); - $absPath = $beleg - ? dirname(__DIR__, 2) . '/public/' . $beleg['storage_path'] + $absPath = ($beleg && !empty($beleg['storage_path'])) + ? Beleg::absolutePath($beleg) : ''; if ($absPath && is_file($absPath)) { header('Content-Type: application/pdf'); @@ -322,17 +323,17 @@ class InvoiceController extends BaseController if ($invoice['status'] !== 'Entwurf' && $beleg !== null) { if (!empty($beleg['storage_path'])) { $relPath = $beleg['storage_path']; - $absDir = dirname(dirname(__DIR__, 2) . '/public/' . $relPath); + $absDir = dirname(Beleg::absolutePath($beleg)); } else { $year = date('Y', strtotime($invoice['rechnungs_datum'])); - $relDir = 'static/uploads/' . $year . '/' . $beleg['beleg_nr']; + $relDir = $year . '/' . $beleg['beleg_nr']; $relPath = $relDir . '/original.pdf'; - $absDir = dirname(__DIR__, 2) . '/public/' . $relDir; + $absDir = Config::get('upload_dir') . '/' . $relDir; } if (!is_dir($absDir)) { mkdir($absDir, 0755, true); } - $absFilePath = dirname(__DIR__, 2) . '/public/' . $relPath; + $absFilePath = Beleg::absolutePath(['storage_path' => $relPath]); file_put_contents($absFilePath, $pdf); \Steuerkram\Database::execute( 'UPDATE belege SET storage_path=?, filename=?, file_hash=?, updated_at=NOW() WHERE id=?', @@ -512,13 +513,13 @@ class InvoiceController extends BaseController $belegNr = Beleg::generateBelegNr(); $year = date('Y', strtotime($invoice['rechnungs_datum'])); - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir)) mkdir($absDir, 0755, true); $filename = $invoice['rechnungs_nr'] . '.pdf'; $storagePath = "{$relDir}/original.pdf"; - file_put_contents(dirname(__DIR__, 2) . "/public/{$storagePath}", $pdf); + file_put_contents(Config::get('upload_dir') . "/{$storagePath}", $pdf); $belegId = Beleg::create([ 'invoice_id' => $id, diff --git a/src/Models/Beleg.php b/src/Models/Beleg.php index edaa37c..c0362d1 100644 --- a/src/Models/Beleg.php +++ b/src/Models/Beleg.php @@ -4,10 +4,21 @@ declare(strict_types=1); namespace Steuerkram\Models; +use Steuerkram\Config; use Steuerkram\Database; class Beleg { + // Bestandsdaten mit altem, webroot-relativem Präfix "static/uploads/" bleiben lesbar. + public static function absolutePath(array $beleg): string + { + $relPath = (string)($beleg['storage_path'] ?? ''); + if (str_starts_with($relPath, 'static/uploads/')) { + $relPath = substr($relPath, strlen('static/uploads/')); + } + return Config::get('upload_dir') . '/' . $relPath; + } + public static function all(array $filter = [], int $page = 1, int $perPage = 30): array { $where = ['1=1']; @@ -250,7 +261,7 @@ class Beleg if ($cnt && $cnt['cnt'] > 0) return false; // Delete file (Datei-Löschung darf DB-Löschung nicht durch Warnings/Header-Fehler stören) - $fullPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $fullPath = self::absolutePath($beleg); if (file_exists($fullPath) && !@unlink($fullPath)) { error_log('Beleg::delete: Datei konnte nicht gelöscht werden: ' . $fullPath); } @@ -300,7 +311,7 @@ class Beleg $beleg = self::find($id); if (!$beleg) return false; - $fullPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $fullPath = self::absolutePath($beleg); if (!file_exists($fullPath)) return false; return hash_file('sha256', $fullPath) === $beleg['file_hash']; diff --git a/src/Services/KontierungsvorschlagService.php b/src/Services/KontierungsvorschlagService.php index 27a6dc7..ef15353 100644 --- a/src/Services/KontierungsvorschlagService.php +++ b/src/Services/KontierungsvorschlagService.php @@ -26,7 +26,7 @@ class KontierungsvorschlagService public function extractText(array $beleg): string { - $absPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $absPath = Beleg::absolutePath($beleg); if (!file_exists($absPath)) return ''; $mime = strtolower($beleg['mime_type'] ?? ''); diff --git a/src/Services/NextcloudImportService.php b/src/Services/NextcloudImportService.php index 4afd127..8f0d1b3 100644 --- a/src/Services/NextcloudImportService.php +++ b/src/Services/NextcloudImportService.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Services; +use Steuerkram\Config; use Steuerkram\Database; use Steuerkram\Models\Beleg; use Steuerkram\Models\EventLog; @@ -260,8 +261,8 @@ class NextcloudImportService // In Upload-Verzeichnis ablegen $belegNr = Beleg::generateBelegNr(); $year = date('Y'); - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir) && !mkdir($absDir, 0755, true)) { $msg = "FEHLER: Verzeichnis konnte nicht erstellt werden: {$absDir}"; @@ -273,7 +274,7 @@ class NextcloudImportService } $storagePath = "{$relDir}/original.{$ext}"; - $absPath = dirname(__DIR__, 2) . "/public/{$storagePath}"; + $absPath = Config::get('upload_dir') . "/{$storagePath}"; if (!rename($tmpPath, $absPath)) { $msg = "FEHLER: Datei konnte nicht verschoben werden: {$filename}"; diff --git a/tests/49_BelegDeleteUnlinkFailTest.php b/tests/49_BelegDeleteUnlinkFailTest.php index aff945a..fb7947f 100644 --- a/tests/49_BelegDeleteUnlinkFailTest.php +++ b/tests/49_BelegDeleteUnlinkFailTest.php @@ -10,7 +10,7 @@ class BelegDeleteUnlinkFailTest extends TestCase public function run(): void { - $dir = dirname(__DIR__) . '/public/static/uploads/test_49_unlink_fail'; + $dir = \Steuerkram\Config::get('upload_dir') . '/test_49_unlink_fail'; $file = $dir . '/original.pdf'; mkdir($dir, 0755, true); @@ -23,7 +23,7 @@ class BelegDeleteUnlinkFailTest extends TestCase $id = Beleg::create([ 'beleg_nr' => 'BLG-TEST-49', 'filename' => 'original.pdf', - 'storage_path' => 'static/uploads/test_49_unlink_fail/original.pdf', + 'storage_path' => 'test_49_unlink_fail/original.pdf', 'mime_type' => 'application/pdf', 'file_hash' => 'testhash49', 'beleg_datum' => '2026-01-01',