fix: close the path-escape hole in delivery (gallery and filename)
The wiki flagged this twice as open: `recipe.picdrop_gallery` had the same
weakness the print module already fixed. `posixpath.join('/kunden/lea',
'../../../etc/cron.d')` is `/etc/cron.d`, and the upload creates the directory
first. Gallery and file names come from the database -- recipes, folders,
settings -- and therefore ultimately from user input.
Checked in `safeSegment()` immediately before the write, in both `uploadBuffer`
(Picdrop and every FTP target) and `uploadToNas`. That is the one place nobody
delivering can bypass, rather than a check somewhere further up that a new call
site would miss.
Deliberately a denylist of the dangerous constructs -- separators, control
characters, all-dot names -- not a narrow allowlist. A customer folder may well
be called "Müller & Söhne (2026)"; that is harmless, and rejecting it would
have been a self-inflicted outage. Covered both ways by tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BNhLunz586g6DeYMa7Qsd1
This commit is contained in:
3 files changed
+60
-6
No files matched your search
+6
-2
@@ -45,8 +45,12 @@ export async function testNas(cfg: NasCfg): Promise<{ ok: boolean; message: stri
|
||||
}
|
||||
|
||||
async function uploadToNas(cfg: NasCfg, remoteDir: string, filename: string, buf: Buffer): Promise<void> {
|
||||
const dir = posixpath.join(cfg.basePath || '/', remoteDir);
|
||||
const finalPath = posixpath.join(dir, filename);
|
||||
// Gleiche Prüfung wie bei Picdrop, aus demselben Grund: `posixpath.join` schützt
|
||||
// nicht vor `..` — der Dateiname kommt aus der Datenbank und wird deshalb hier,
|
||||
// unmittelbar vor dem Schreiben, geprüft.
|
||||
const { safeSegment } = await import('./picdrop');
|
||||
const dir = posixpath.join(cfg.basePath || '/', safeSegment(remoteDir, 'Galerie'));
|
||||
const finalPath = posixpath.join(dir, safeSegment(filename, 'Dateiname'));
|
||||
const { Readable } = await import('node:stream');
|
||||
if (cfg.protocol === 'sftp') {
|
||||
const SftpClient = (await import('ssh2-sftp-client')).default;
|
||||
|
||||
+28
-4
@@ -1,8 +1,8 @@
|
||||
// Picdrop-Auslieferung via SFTP (ssh2-sftp-client) oder FTPS (basic-ftp).
|
||||
// Konfiguration kommt aus settings (verschlüsseltes Passwort).
|
||||
import posixpath from 'node:path/posix';
|
||||
import { one } from './db';
|
||||
import { decrypt } from './crypto';
|
||||
import { one } from './db.ts';
|
||||
import { decrypt } from './crypto.ts';
|
||||
|
||||
export interface PicdropCfg {
|
||||
host: string; protocol: 'ftps' | 'sftp'; port: number;
|
||||
@@ -58,11 +58,35 @@ export async function testConnection(cfg: PicdropCfg): Promise<{ ok: boolean; me
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ein Pfadstück, das gefahrlos an den Basisordner angehängt werden kann.
|
||||
*
|
||||
* `posixpath.join` ist **kein** Schutz: `join('/kunden/lea', '../../../etc/cron.d')`
|
||||
* ergibt `/etc/cron.d`, und der Upload legt das Verzeichnis vorher an. Galerienamen
|
||||
* und Dateinamen stammen aus der Datenbank (Rezepte, Ordner, Einstellungen) und
|
||||
* damit letztlich aus Nutzereingaben — sie werden deshalb **unmittelbar vor dem
|
||||
* Schreiben** geprüft, nicht irgendwo weiter oben. Diese Stelle kann niemand
|
||||
* umgehen, der ausliefert.
|
||||
*/
|
||||
export function safeSegment(value: string, was: 'Galerie' | 'Dateiname'): string {
|
||||
const v = String(value || '').trim();
|
||||
if (!v) return '';
|
||||
// Bewusst eine Sperrliste der *gefährlichen* Konstrukte statt einer engen
|
||||
// Erlaubnisliste: Ein Kundenordner darf „Müller & Söhne (2026)" heißen — das ist
|
||||
// harmlos. Gefährlich sind nur Trennzeichen, Steuerzeichen und reine Punktnamen,
|
||||
// denn allein damit lässt sich der Basisordner verlassen.
|
||||
if (/[\/\\]/.test(v)) throw new Error(`Ungültiger ${was}: keine Schrägstriche erlaubt.`);
|
||||
if (/[\u0000-\u001f\u007f]/.test(v)) throw new Error(`Ungültiger ${was}: Steuerzeichen.`);
|
||||
if (/^\.+$/.test(v)) throw new Error(`Ungültiger ${was}: „.“ und „..“ sind keine Namen.`);
|
||||
if (v.length > 128) throw new Error(`Ungültiger ${was}: zu lang.`);
|
||||
return v;
|
||||
}
|
||||
|
||||
/** Lädt einen Puffer direkt unter dem finalen Namen hoch.
|
||||
* (Picdrop verträgt das Temp→Rename-Muster nicht — es blieben `.tmp-…`-Dateien liegen.) */
|
||||
export async function uploadBuffer(cfg: PicdropCfg, gallery: string, filename: string, buf: Buffer): Promise<void> {
|
||||
const dir = posixpath.join(cfg.basePath || '/', gallery || '');
|
||||
const finalPath = posixpath.join(dir, filename);
|
||||
const dir = posixpath.join(cfg.basePath || '/', safeSegment(gallery, 'Galerie'));
|
||||
const finalPath = posixpath.join(dir, safeSegment(filename, 'Dateiname'));
|
||||
const { Readable } = await import('node:stream');
|
||||
|
||||
if (cfg.protocol === 'sftp') {
|
||||
|
||||
Reference in new issue
Block a user