361 lines
10 KiB
Markdown
361 lines
10 KiB
Markdown
# Security Audit Report: Stundenerfassung
|
||
|
||
**Datum:** 2026-03-31
|
||
**Geprüft von:** Claude Code (automatisierte Analyse)
|
||
**Scope:** Alle API-Routes, Middleware, Services, EJS-Templates, Checkin-Server
|
||
|
||
---
|
||
|
||
## Zusammenfassung
|
||
|
||
| Priorität | Anzahl | Beschreibung |
|
||
|-----------|--------|--------------|
|
||
| P0 - Vor Go-Live fixen | 4 | Session-Secret, Default-Credentials, Checkin ohne Auth, kein CSRF |
|
||
| P1 - Bald fixen | 4 | XSS (mehrere Stellen), Session-Cookie-Flags, kein Rate-Limiting |
|
||
| P2 - Sollte gefixt werden | 3 | Error-Message-Leaks, fehlende Security-Headers, JS-String-Injection |
|
||
| P3 - Nice to have | 1 | LDAP-Passwort unverschlüsselt in DB |
|
||
|
||
---
|
||
|
||
## 1. SQL Injection
|
||
|
||
### Ergebnis: KEINE Schwachstellen gefunden
|
||
|
||
Alle SQLite-Queries verwenden konsequent parametrisierte Queries (`?` Platzhalter) in allen Route-Dateien:
|
||
|
||
```js
|
||
db.get('SELECT * FROM users WHERE username = ? COLLATE NOCASE', [username], ...)
|
||
db.run('INSERT INTO users (...) VALUES (?, ?, ?, ...)', [values...], ...)
|
||
```
|
||
|
||
Die MSSQL-Projektsuche (`services/mssql-infra-service.js:53`) verwendet ebenfalls korrekt parametrisierte Eingaben:
|
||
|
||
```js
|
||
request.input('search', sql.NVarChar, `%${searchTerm.toUpperCase()}%`);
|
||
```
|
||
|
||
Die dynamischen `IN (${placeholders})`-Konstruktionen in `routes/admin-routes.js:475` und `routes/verwaltung-routes.js:1108` sind sicher, da die Platzhalter aus `userIds.map(() => '?')` generiert werden.
|
||
|
||
---
|
||
|
||
## 2. XSS (Cross-Site Scripting)
|
||
|
||
### 2.1 CRITICAL: onclick-Handler-Injection in admin.ejs
|
||
|
||
**Datei:** `views/admin.ejs` (ca. Zeile 213)
|
||
|
||
```html
|
||
<button onclick="deleteUser(<%= u.id %>, '<%= u.username %>')">Löschen</button>
|
||
```
|
||
|
||
**Problem:** Ein Username wie `'); alert(document.cookie);//` bricht aus dem String aus und führt JavaScript im Admin-Kontext aus.
|
||
|
||
**Fix:** `onclick`-Handler durch Event-Listener ersetzen und Daten über `data-*`-Attribute übergeben:
|
||
|
||
```html
|
||
<button class="btn-delete" data-id="<%= u.id %>" data-username="<%= u.username %>">Löschen</button>
|
||
```
|
||
|
||
```js
|
||
document.querySelectorAll('.btn-delete').forEach(btn => {
|
||
btn.addEventListener('click', () => {
|
||
deleteUser(btn.dataset.id, btn.dataset.username);
|
||
});
|
||
});
|
||
```
|
||
|
||
---
|
||
|
||
### 2.2 HIGH: innerHTML mit unescapten Daten (4 Stellen)
|
||
|
||
#### a) `views/project-search.ejs` (ca. Zeile 107-114)
|
||
|
||
```js
|
||
tr.innerHTML = `
|
||
<td>${auftrag}</td>
|
||
<td>${proj}</td>
|
||
<td>${such}</td>
|
||
<td>${knd}</td>
|
||
<td>${bez}</td>
|
||
`;
|
||
```
|
||
|
||
**Problem:** MSSQL-Suchergebnisse werden direkt in innerHTML eingefügt. Falls die externe Datenbank manipulierte Daten enthält, wird HTML/JS ausgeführt.
|
||
|
||
**Fix:** `textContent` statt `innerHTML` verwenden:
|
||
|
||
```js
|
||
const tr = document.createElement('tr');
|
||
[auftrag, proj, such, knd, bez].forEach(val => {
|
||
const td = document.createElement('td');
|
||
td.textContent = val;
|
||
tr.appendChild(td);
|
||
});
|
||
```
|
||
|
||
#### b) `views/verwaltung.ejs` (ca. Zeile 871-876)
|
||
|
||
```js
|
||
li.innerHTML = `Korrektur am ${dateText} <span class="${hoursClass}">${hoursDisplay}</span> – ${reason}`;
|
||
```
|
||
|
||
**Problem:** Das `reason`-Feld aus der Tabelle `overtime_corrections` wird ohne Escaping eingefügt. Jeder Verwaltungs-User kann beim Speichern einer Korrektur HTML/JS injizieren.
|
||
|
||
**Fix:** `reason` mit einer Escape-Funktion behandeln oder `textContent` verwenden.
|
||
|
||
#### c) `views/overtime-breakdown.ejs` (ca. Zeile 516-520)
|
||
|
||
Identisches Problem wie b) - gleiches `reason`-Feld, gleiche innerHTML-Nutzung.
|
||
|
||
#### d) `views/project-search.ejs` - data-Attribut
|
||
|
||
```js
|
||
data-auftrag="${auftrag}"
|
||
```
|
||
|
||
**Problem:** Unescapte Daten in HTML-Attributen können aus dem Attribut ausbrechen.
|
||
|
||
---
|
||
|
||
### 2.3 MEDIUM: JavaScript-String-Literal-Injection
|
||
|
||
**Datei:** `views/verwaltung.ejs` (ca. Zeile 337)
|
||
|
||
```js
|
||
const currentUser = '<%= user.firstname %> <%= user.lastname %>';
|
||
```
|
||
|
||
**Problem:** EJS `<%=` escaped HTML-Entities (`<`, `>`, `&`, `"`), aber NICHT einfache Anführungszeichen (`'`) oder Backslashes in JS-Kontext. Ein Name mit `'` bricht den String.
|
||
|
||
**Fix:** `JSON.stringify()` verwenden:
|
||
|
||
```js
|
||
const currentUser = <%- JSON.stringify((user.firstname || '') + ' ' + (user.lastname || '')) %>;
|
||
```
|
||
|
||
---
|
||
|
||
## 3. Authentication & Session Security
|
||
|
||
### 3.1 CRITICAL (P0): Hardcoded Session Secret
|
||
|
||
**Datei:** `server.js:33`
|
||
|
||
```js
|
||
secret: 'stundenerfassung-geheim-2024'
|
||
```
|
||
|
||
**Problem:** Jeder, der den Quellcode kennt, kann Session-Cookies fälschen und sich als beliebiger User ausgeben.
|
||
|
||
**Fix:** Umgebungsvariable verwenden:
|
||
|
||
```js
|
||
secret: process.env.SESSION_SECRET || require('crypto').randomBytes(64).toString('hex')
|
||
```
|
||
|
||
Und in der Produktionsumgebung `SESSION_SECRET` als Environment-Variable setzen.
|
||
|
||
---
|
||
|
||
### 3.2 CRITICAL (P0): Default-Credentials
|
||
|
||
**Datei:** `server.js:91-92` (und `database.js` bei der Initialisierung)
|
||
|
||
```
|
||
Admin: admin / admin123
|
||
Verwaltung: verwaltung / verwaltung123
|
||
```
|
||
|
||
**Problem:** Falls diese Accounts noch mit den Standard-Passwörtern existieren, kann jeder sich als Admin anmelden.
|
||
|
||
**Fix:**
|
||
- Standard-Passwörter nach erster Anmeldung erzwingen zu ändern
|
||
- Oder: Standard-Accounts bei Produktion entfernen / Passwort aus Env-Variable lesen
|
||
- Mindestens: Konsolenausgabe der Credentials in Produktion deaktivieren
|
||
|
||
---
|
||
|
||
### 3.3 CRITICAL (P0): Kein CSRF-Schutz
|
||
|
||
**Betrifft:** Alle POST/PUT/DELETE-Routen
|
||
|
||
**Problem:** Keine CSRF-Tokens auf Formularen oder API-Aufrufen. Ein Angreifer kann eine bösartige Webseite erstellen, die im Namen eines eingeloggten Admins Aktionen ausführt (User erstellen, Timesheets löschen, Einstellungen ändern).
|
||
|
||
**Fix:** CSRF-Middleware installieren:
|
||
|
||
```bash
|
||
npm install csurf
|
||
```
|
||
|
||
```js
|
||
const csrf = require('csurf');
|
||
app.use(csrf());
|
||
```
|
||
|
||
Oder alternativ `SameSite=Strict` auf dem Session-Cookie setzen (einfacher, aber weniger robust).
|
||
|
||
---
|
||
|
||
### 3.4 HIGH (P1): Session-Cookie ohne Sicherheits-Flags
|
||
|
||
**Datei:** `server.js:33-37`
|
||
|
||
```js
|
||
cookie: { maxAge: 24 * 60 * 60 * 1000 }
|
||
```
|
||
|
||
**Problem:** Fehlende Flags:
|
||
- `httpOnly: true` - Verhindert JS-Zugriff auf Cookie (wichtig gegen XSS)
|
||
- `secure: true` - Cookie nur über HTTPS senden
|
||
- `sameSite: 'strict'` - Schutz gegen CSRF
|
||
|
||
**Fix:**
|
||
|
||
```js
|
||
cookie: {
|
||
maxAge: 24 * 60 * 60 * 1000,
|
||
httpOnly: true,
|
||
secure: process.env.NODE_ENV === 'production',
|
||
sameSite: 'strict'
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 3.5 HIGH (P1): Kein Rate-Limiting auf Login
|
||
|
||
**Datei:** `routes/auth-routes.js` - POST `/login`
|
||
|
||
**Problem:** Unbegrenzte Login-Versuche ermöglichen Brute-Force-Angriffe auf Passwörter.
|
||
|
||
**Fix:**
|
||
|
||
```bash
|
||
npm install express-rate-limit
|
||
```
|
||
|
||
```js
|
||
const rateLimit = require('express-rate-limit');
|
||
const loginLimiter = rateLimit({
|
||
windowMs: 15 * 60 * 1000, // 15 Minuten
|
||
max: 10, // Max 10 Versuche
|
||
message: { error: 'Zu viele Anmeldeversuche. Bitte in 15 Minuten erneut versuchen.' }
|
||
});
|
||
app.post('/login', loginLimiter, (req, res) => { ... });
|
||
```
|
||
|
||
---
|
||
|
||
## 4. Checkin-Server (Port 3334)
|
||
|
||
### CRITICAL (P0): Komplett unauthentifizierte Endpoints
|
||
|
||
**Datei:** `checkin-server.js:44,110`
|
||
|
||
```
|
||
GET /api/checkin/:userId
|
||
GET /api/checkout/:userId
|
||
```
|
||
|
||
**Problem:** JEDER, der eine User-ID kennt (oder errät), kann beliebige Mitarbeiter ein- und auschecken. User-IDs sind sequentielle Integers ab 1 - Enumeration ist trivial.
|
||
|
||
**Fix-Optionen:**
|
||
1. **Token-basiert:** QR-Codes enthalten einen signierten Token statt nur die User-ID
|
||
2. **HMAC-Signatur:** `/api/checkin/:userId?token=HMAC(userId, secret)` - Server validiert Token
|
||
3. **IP-Beschränkung:** Checkin-Server nur aus dem internen Netzwerk erreichbar machen (Firewall)
|
||
4. **Mindestens:** UUID statt sequentieller IDs für den Checkin-Endpunkt verwenden
|
||
|
||
---
|
||
|
||
## 5. Information Disclosure
|
||
|
||
### 5.1 HIGH (P2): Fehlermeldungen leaken interne Details
|
||
|
||
**Datei:** `routes/admin-routes.js:334`
|
||
|
||
```js
|
||
error: 'Verbindung zur MSSQL-Datenbank fehlgeschlagen: ' + err.message
|
||
```
|
||
|
||
**Datei:** `routes/timesheet-routes.js:248`
|
||
|
||
```js
|
||
error: 'Fehler beim Speichern: ' + err.message
|
||
```
|
||
|
||
**Problem:** MSSQL-Fehlermeldungen können Servernamen, IP-Adressen und Datenbankstruktur leaken. SQLite-Fehler können Tabellen-/Spaltennamen verraten.
|
||
|
||
**Fix:** Generische Fehlermeldungen an den Client senden, Details nur serverseitig loggen:
|
||
|
||
```js
|
||
console.error('MSSQL Fehler:', err);
|
||
return res.status(500).json({ error: 'Datenbankverbindung fehlgeschlagen' });
|
||
```
|
||
|
||
---
|
||
|
||
### 5.2 MEDIUM (P3): LDAP-Bind-Passwort unverschlüsselt in DB
|
||
|
||
**Tabelle:** `ldap_config.bind_password`
|
||
|
||
**Problem:** Das LDAP-Bind-Passwort wird im Klartext in der SQLite-Datenbank gespeichert. Der GET-Endpoint (`routes/admin-ldap-routes.js:18`) entfernt es zwar aus der Antwort, aber es liegt im Klartext in der DB-Datei.
|
||
|
||
**Fix:** Passwort verschlüsselt speichern (z.B. mit `crypto.createCipheriv`) und bei Bedarf entschlüsseln.
|
||
|
||
---
|
||
|
||
## 6. Fehlende Security-Headers
|
||
|
||
### P2: Keine globalen Sicherheits-Header
|
||
|
||
**Datei:** `server.js`
|
||
|
||
**Fehlende Header:**
|
||
- `X-Content-Type-Options: nosniff`
|
||
- `X-Frame-Options: DENY`
|
||
- `Content-Security-Policy`
|
||
- `Strict-Transport-Security` (HSTS)
|
||
- `Referrer-Policy: strict-origin-when-cross-origin`
|
||
|
||
**Fix:** `helmet` Middleware installieren:
|
||
|
||
```bash
|
||
npm install helmet
|
||
```
|
||
|
||
```js
|
||
const helmet = require('helmet');
|
||
app.use(helmet());
|
||
```
|
||
|
||
Das setzt automatisch alle empfohlenen Security-Headers.
|
||
|
||
---
|
||
|
||
## 7. Positiv aufgefallen
|
||
|
||
- Alle SQL-Queries parametrisiert (kein SQL-Injection-Risiko)
|
||
- PDF-Pfad-Traversal korrekt verhindert (`resolveWithinBaseDir()`, Regex-Validierung)
|
||
- Passwörter mit bcrypt gehasht
|
||
- Rollen-System korrekt implementiert (requireAuth, requireAdmin, requireVerwaltung)
|
||
- EJS `<%=` (escaped) wird konsequent statt `<%-` (unescaped) verwendet
|
||
- LDAP-Passwort wird im GET-Endpoint korrekt entfernt
|
||
- `X-Content-Type-Options: nosniff` wird auf PDF-Responses gesetzt
|
||
|
||
---
|
||
|
||
## Checkliste zum Abarbeiten
|
||
|
||
- [ ] **P0** Session-Secret aus Env-Variable lesen (`server.js`)
|
||
- [ ] **P0** Default-Credentials entfernen oder Passwort-Änderung erzwingen
|
||
- [ ] **P0** Checkin-Server absichern (Token/HMAC/IP-Restriction)
|
||
- [ ] **P0** CSRF-Schutz hinzufügen (csurf oder SameSite-Cookie)
|
||
- [ ] **P1** XSS in admin.ejs fixen (onclick -> addEventListener)
|
||
- [ ] **P1** innerHTML durch textContent/DOM-Methoden ersetzen (4 Stellen)
|
||
- [ ] **P1** Session-Cookie-Flags setzen (httpOnly, secure, sameSite)
|
||
- [ ] **P1** Rate-Limiting auf Login-Route
|
||
- [ ] **P2** Fehlermeldungen generisch machen (keine err.message an Client)
|
||
- [ ] **P2** Security-Headers via helmet setzen
|
||
- [ ] **P2** JS-String-Literal-Injection fixen (JSON.stringify)
|
||
- [ ] **P3** LDAP-Passwort verschlüsselt speichern
|