Command, Path and Template Injection

import { execFile } from 'node:child_process';
import { promisify } from 'node:util';
import path from 'node:path';
import crypto from 'node:crypto';

const run = promisify(execFile);
const WORK_DIR = '/var/app/work';

export async function generateThumbnail(uploadId, userId) {
  // 1. The path comes from OUR database, never from the request.
  const upload = await db('uploads').where({ id: uploadId, user_id: userId }).first();
  if (!upload) throw new NotFoundError();

  // 2. Storage keys are generated, not derived from the user's filename.
  const input = path.join(WORK_DIR, upload.storage_key);
  const output = path.join(WORK_DIR, `${crypto.randomUUID()}.jpg`);

  // 3. Confirm both paths are still inside the base directory.
  for (const p of [input, output]) {
    if (!path.resolve(p).startsWith(path.resolve(WORK_DIR) + path.sep)) {
      throw new Error('path escapes the working directory');
    }
  }

  // 4. execFile: an ARGUMENT ARRAY, no shell, so nothing is interpreted.
  try {
    await run('convert', [
      `${input}[0]`,               // first frame only — bounds animated inputs
      '-resize', '400x400>',
      '-strip',                    // drop EXIF, which can carry payloads
      '-quality', '85',
      output,
    ], {
      timeout: 30_000,             // 5. never let it hang
      maxBuffer: 1024 * 1024,      // 6. bound the output
      killSignal: 'SIGKILL',
      env: {},                     // 7. an empty environment: no secrets leak in
      cwd: WORK_DIR,
    });
  } catch (err) {
    // 8. Never return the tool's stderr — it contains paths and versions.
    logger.error({ err, uploadId }, 'thumbnail generation failed');
    throw new UnprocessableError('could not process this image');
  }

  return output;
}

// ⚠️ Even done correctly, invoking a media library on untrusted input is
// risky — ImageMagick and ffmpeg have a long CVE history. Run it in a
// sandbox: a locked-down container, no network, read-only root, dropped
// capabilities, and a memory limit.

import path from 'node:path';
import fs from 'node:fs/promises';

const STORAGE_ROOT = '/var/app/uploads';

// ❌ Every one of these is traversable
res.sendFile(path.join(STORAGE_ROOT, req.params.filename));
res.sendFile(STORAGE_ROOT + '/' + req.query.file);
// ../../../etc/passwd
// ..%2f..%2f..%2fetc%2fpasswd          ← url encoded
// ....//....//etc/passwd               ← defeats a naive '../' replace
// /etc/passwd                          ← absolute path wins in path.join? no,
//                                        but path.resolve would

// ✅ Best: never accept a path at all. Look it up by id.
app.get('/api/files/:id', auth, async (req, res) => {
  const file = await db('files')
    .where({ id: req.params.id, user_id: req.user.id })   // authorization
    .first();
  if (!file) return res.status(404).json({ error: 'not_found' });

  // storage_key was generated by us at upload time: a uuid, no user input.
  const absolute = path.join(STORAGE_ROOT, file.storage_key);

  res.type(file.mime_type)
     .set('Content-Disposition',
          `attachment; filename="${sanitiseFilename(file.original_name)}"`)
     .sendFile(absolute);
});

// ✅ If you genuinely must accept a name, resolve and then VERIFY containment.
export function safeJoin(base, userPath) {
  const resolvedBase = path.resolve(base);
  const target = path.resolve(resolvedBase, userPath);

  // The separator matters: '/var/app/uploads-evil' also startsWith('/var/app/uploads')
  if (target !== resolvedBase && !target.startsWith(resolvedBase + path.sep)) {
    throw new ForbiddenError('path traversal');
  }
  return target;
}

// ...and check the resolved path AFTER following symlinks, or a symlink inside
// the directory points wherever it likes.
const real = await fs.realpath(safeJoin(STORAGE_ROOT, name));
if (!real.startsWith(path.resolve(STORAGE_ROOT) + path.sep)) {
  throw new ForbiddenError('symlink escapes storage root');
}

// And the header sanitiser, because a filename with a quote or newline in it
// is header injection:
const sanitiseFilename = (n) =>
  n.replace(/[^\w.\- ]/g, '_').slice(0, 100);

// ❌ User input COMPILED as a template. This is remote code execution.
import Handlebars from 'handlebars';
const template = Handlebars.compile(user.emailTemplate);   // attacker-authored
const html = template({ user });

// In many engines the payload reaches the runtime:
//   {{#with "s" as |string|}}{{#with split as |conslist|}}...{{/with}}{{/with}}
//   {{constructor.constructor('return process.env')()}}
// Nunjucks, Pug, EJS, Jinja2, Twig and Freemarker all have known escapes.

// ✅ Templates come from your codebase. User input is a VALUE.
const template = Handlebars.compile(
  await fs.readFile('templates/invoice-email.hbs', 'utf8'));
const html = template({
  customerName: user.name,        // escaped by the engine on output
  total: formatMoney(invoice.totalCents),
});

// ✅ If users genuinely need customisation, give them a value allowlist
// with a logic-less engine and simple placeholder substitution.
const ALLOWED_VARS = ['customer_name', 'invoice_number', 'total', 'due_date'];

export function renderUserTemplate(templateText, values) {
  // Bound the input first
  if (templateText.length > 10_000) throw new BadRequestError('template too long');

  // Reject anything that is not a plain {{ placeholder }}
  const used = [...templateText.matchAll(/\{\{\s*([\w.]+)\s*\}\}/g)].map((m) => m[1]);
  const unknown = used.filter((v) => !ALLOWED_VARS.includes(v));
  if (unknown.length) {
    throw new BadRequestError(`unknown variables: ${unknown.join(', ')}`);
  }

  // Anything that is not a recognised placeholder must not survive
  if (/\{\{[^}]*[^\w.\s}][^}]*\}\}/.test(templateText)) {
    throw new BadRequestError('only simple {{ variable }} placeholders are allowed');
  }

  // Plain substitution — no engine, no compilation, no evaluation.
  return templateText.replace(/\{\{\s*([\w.]+)\s*\}\}/g,
    (_, name) => escapeHtml(String(values[name] ?? '')));
}

// The principle: the engine never sees attacker-controlled template text.
// Substitution is a string operation, not an evaluation.