Path traversal in storage import allows writing files outside storage directory

[sulthonzh]

Bug: Path traversal via tar import in StorageService

Reproduction

// test-path-traversal.js
// Requires: npm install tar-stream
const path = require("path");
const fs = require("fs");
const tar = require("tar-stream");

// Simulate the vulnerable putLocalFile from storage.service.ts
const localPath = "./data/media";
function putLocalFile(filePath, data) {
  const fullPath = path.join(localPath, filePath);
  const dir = path.dirname(fullPath);
  if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true });
  fs.writeFileSync(fullPath, data);
  return fullPath;
}

// Craft a tar with path traversal entry
const pack = tar.pack();
pack.entry({ name: "../../outside-storage.txt" }, "written outside storage dir");
pack.finalize();

// Simulate importFromStream extraction
const extract = tar.extract();
extract.on("entry", (header, stream, next) => {
  const chunks = [];
  stream.on("data", (c) => chunks.push(c));
  stream.on("end", () => {
    const written = putLocalFile(header.name, Buffer.concat(chunks));
    console.log("Entry:", header.name);
    console.log("Written to:", path.resolve(written));
    console.log("Escapes?", !path.resolve(written).startsWith(path.resolve(localPath)));
    next();
  });
  stream.resume();
});

pack.pipe(extract);

$ node test-path-traversal.js
Entry: ../../outside-storage.txt
Written to: /private/tmp/outside-storage.txt
Escapes? true

Expected: Files with .. in the path should be rejected or normalized to stay within the storage directory.

Actual: Files are written outside the storage directory, allowing arbitrary file write on the server.

Environment

• Version: 0fbee7f (latest main)
• Node: 22 LTS
• OS: macOS (arm64)

Root Cause

In src/common/storage/storage.service.ts, the putLocalFile method (line 255) joins the user-controlled filePath with this.localPath using path.join() without validating that the resolved path stays within the base directory:

private putLocalFile(filePath: string, data: Buffer): Promise<void> {
    const fullPath = path.join(this.localPath, filePath);  // filePath from tar header.name
    // ... writes file without checking if fullPath escapes localPath

The filePath comes from importFromStream (line 192), which passes header.name directly from tar entries without sanitization. An attacker who can reach the POST /infra/storage/import endpoint (requires a valid API key) can craft a tar.gz archive containing entries with ../../ paths to write files anywhere the process has filesystem access.

The same issue affects getLocalFile (line 250) — a crafted tar import followed by a list/get cycle could read arbitrary files.

Suggested Fix

Sanitize file paths in putLocalFile and getLocalFile by resolving against the base directory and checking the result stays within it:

private sanitizePath(filePath: string): string {
    const resolved = path.resolve(this.localPath, filePath);
    if (!resolved.startsWith(path.resolve(this.localPath) + path.sep) && resolved !== path.resolve(this.localPath)) {
        throw new Error(`Path traversal detected: ${filePath}`);
    }
    return resolved;
}

private getLocalFile(filePath: string): Promise<Buffer> {
    const fullPath = this.sanitizePath(filePath);
    return Promise.resolve(fs.readFileSync(fullPath));
}

private putLocalFile(filePath: string, data: Buffer): Promise<void> {
    const fullPath = this.sanitizePath(filePath);
    const dir = path.dirname(fullPath);
    if (!fs.existsSync(dir)) {
        fs.mkdirSync(dir, { recursive: true });
    }
    fs.writeFileSync(fullPath, data);
    return Promise.resolve();
}

Additionally, in importFromStream, consider rejecting or skipping entries with .. in their name before calling putFile.