FDS2610/Steuerkram
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix #116: Beleg-Uploads aus dem Web-Root entfernt (nach data/uploads verschoben)

Browse source
This commit is contained in:
fds2610-bot 2026-07-08 07:37:45 +02:00
parent 2012f228c4
commit 649684a77b
13 changed files with 56 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
Changes.md
View file

@ -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.

3
Dockerfile
View file

@ -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

2
docker-compose.yml
View file

@ -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

2
docker-entrypoint.sh
View file

@ -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

2
docs/datenbankstruktur.md
View file

@ -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 |

2
src/Config.php
View file

@ -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' => '',
];
}

7
src/Controllers/BankController.php
View file

@ -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);

13
src/Controllers/BelegController.php
View file

@ -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.']);

19
src/Controllers/InvoiceController.php
View file

@ -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,

15
src/Models/Beleg.php
View file

@ -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'];

2
src/Services/KontierungsvorschlagService.php
View file

@ -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'] ?? '');

7
src/Services/NextcloudImportService.php
View file

@ -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}";

4
tests/49_BelegDeleteUnlinkFailTest.php
View file

@ -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',