Path traversal: perché startsWith non basta e come si contiene davvero un percorso

1// ✗ Il filtro a lista di divieti: sembra ragionevole, non regge.2function pulisci(nome) {3  return nome.replace(/\.\.\//g, '');     // toglie i "../"4}5​6pulisci('../../etc/passwd');       // → 'etc/passwd'          sembra funzionare7pulisci('....//....//etc/passwd'); // → '../../etc/passwd'    (!)8// Rimuovendo il "../" centrale, i frammenti rimasti si ricompongono.9// Applicare il replace in ciclo finché non cambia niente chiude questo caso10// e ne lascia aperti altri tre: la codifica multipla, i separatori di11// sistemi diversi e la normalizzazione Unicode.

1// Il primo passo giusto: risolvere, poi verificare. path.resolve() applica2// i ".." in modo puramente lessicale, senza toccare il disco.3const base = path.resolve('/var/www/public');4const target = path.resolve(base, richiesto);5​6if (!target.startsWith(base)) return nega();   // ← quasi giusto

1// ✗ Il difetto di startsWith, che passa inosservato per anni.2const base = '/var/www/public';3​4'/var/www/public/img/logo.png'.startsWith(base);   // true  — corretto5'/var/www/public-backup/dump.sql'.startsWith(base); // true  — SBAGLIATO (!)6'/var/www/publications/segreti.pdf'.startsWith(base); // true — SBAGLIATO (!)7​8// Il confronto è fra stringhe, e "public" è prefisso di "public-backup".9// Basta una cartella fratella dal nome simile e il contenimento non c'è più.10​11// ✓ Due forme corrette. La prima aggiunge il separatore:12const dentro = target === base || target.startsWith(base + path.sep);13​14// ✓ La seconda, preferibile, delega a path.relative: se per raggiungere15//   il target da base bisogna risalire, il target è fuori.16const rel = path.relative(base, target);17const dentro2 = rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel);

1// L'ordine delle operazioni conta più della loro correttezza.2// Il framework decodifica una volta; se il codice decodifica di nuovo,3// la doppia codifica passa il controllo e si riapre dopo.4​5// ✗ Controllo prima, decodifica dopo: il controllo lavora su un dato6//   che non è quello che verrà usato.7const nome = req.query.file;                  // '%2e%2e%2fsegreto'8if (nome.includes('..')) return nega();       // non lo trova9fs.readFile(decodeURIComponent(nome));        // → '../segreto'  (!)10​11// ✓ Decodificare tutto quello che va decodificato, UNA volta,12//   e da lì in poi lavorare solo sul valore risolto.13let nome2;14try {15  nome2 = decodeURIComponent(req.query.file || '');16} catch {17  return nega();                              // percentuale malformata18}19if (nome2.includes('\0')) return nega();      // Node solleva comunque, ma20                                              // meglio un rifiuto esplicito

1// path.resolve() è puramente lessicale: non sa che 'uploads/report.pdf'2// è un link simbolico che punta a /etc/shadow. Il contenimento passa,3// la lettura esce dal perimetro.4const vero = await fs.promises.realpath(target);   // segue i link5if (!contenuto(base, vero)) return nega();6​7// Resta però una finestra TOCTOU: fra il realpath e la open, un processo8// che scrive nella cartella (per esempio un altro utente che carica file)9// può sostituire il percorso con un link.10//11// La forma priva di quella finestra: aprire senza seguire i link, e12// verificare sul descrittore già aperto — non più sul nome.13const fd = await fs.promises.open(target, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW);14try {15  const st = await fd.stat();16  if (!st.isFile()) return nega();17  return fd.createReadStream();18} catch (e) {19  if (e.code === 'ELOOP') return nega();           // era un link simbolico20  throw e;21}

1// La stessa vulnerabilità, in un vestito diverso: i nomi dentro un archivio2// sono input dell'utente esattamente come una querystring.3// Un file .zip può contenere una voce chiamata "../../../etc/cron.d/backdoor".4for (const voce of archivio.voci) {5  const destinazione = path.resolve(CARTELLA_ESTRAZIONE, voce.nome);6​7  // Senza questo controllo, l'estrazione scrive dove vuole l'archivio.8  const rel = path.relative(CARTELLA_ESTRAZIONE, destinazione);9  if (rel.startsWith('..') || path.isAbsolute(rel)) {10    throw new Error(`voce fuori perimetro: ${voce.nome}`);11  }12  if (voce.tipoLink) continue;        // i link simbolici dentro un archivio13                                      // non hanno usi legittimi qui14  await scrivi(destinazione, voce.dati);15}

1// La difesa più solida non è un controllo migliore: è togliere all'utente2// la possibilità di comporre un percorso.3const DOCUMENTI = new Map([4  ['listino-2026',  { file: 'listino-2026.pdf',  nome: 'Listino 2026.pdf' }],5  ['manuale-base',  { file: 'manuale-v3.pdf',    nome: 'Manuale.pdf' }]6]);7​8app.get('/download/:id', (req, res) => {9  const doc = DOCUMENTI.get(req.params.id);10  if (!doc) return res.status(404).end();11​12  // Il percorso non deriva più dall'input: deriva da una tabella che13  // avete scritto voi. Non c'è niente da attraversare.14  res.download(path.join(BASE, doc.file), doc.nome);15});

1// Se un percorso relativo dall'utente è inevitabile, non riscrivete il2// contenimento: Express lo implementa già, ed è codice molto più3// collaudato del vostro.4app.get('/statici/*percorso', (req, res) => {5  res.sendFile(req.params.percorso, {6    root: path.resolve('public'),   // obbligatorio: definisce il perimetro7    dotfiles: 'deny',               // niente .env, .git, .htaccess8    index: false9  }, (err) => {10    if (err) res.status(err.status || 404).end();11  });12});