Fix security vulnerability in archive extraction

src/infra/archive.ts

@@ -3,6 +3,136 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import * as tar from "tar";
 
+/**
+ * Custom error class for path traversal attempts
+ */
+class PathTraversalError extends Error {
+  constructor(
+    message: string,
+    public readonly code: string,
+    public readonly attemptedPath: string,
+  ) {
+    super(message);
+    this.name = "PathTraversalError";
+
+    // Maintains proper stack trace in V8 environments
+    if (Error.captureStackTrace) {
+      Error.captureStackTrace(this, PathTraversalError);
+    }
+  }
+}
+
+/**
+ * Validates a zip entry path to prevent path traversal attacks.
+ * Uses multiple layers of defense to catch various attack vectors.
+ *
+ * @param entryPath - The path from the archive entry
+ * @param destDir - The destination directory for extraction
+ * @returns true if the path is safe, throws PathTraversalError if not
+ */
+function validateZipEntryPath(entryPath: string, destDir: string): string {
+  // Layer 1: Input Sanitization - Reject obviously malicious input
+  if (/\x00/.test(entryPath)) {
+    throw new PathTraversalError(
+      `Path contains null byte: ${entryPath}`,
+      "NULL_BYTE",
+      entryPath,
+    );
+  }
+
+  // Check for control characters that shouldn't appear in paths
+  if (/[\x01-\x08\x0B\x0C\x0E-\x1F]/.test(entryPath)) {
+    throw new PathTraversalError(
+      `Path contains invalid control characters: ${entryPath}`,
+      "CONTROL_CHAR",
+      entryPath,
+    );
+  }
+
+  // Layer 2: Pre-Resolution Validation - Catch traversal before resolve
+  // Normalize path separators to platform-native format for consistent handling
+  const normalizedEntryPath = entryPath.split(/[/\\]/).join(path.sep);
+
+  // Reject absolute paths - zip entries should always be relative
+  if (path.isAbsolute(normalizedEntryPath)) {
+    throw new PathTraversalError(
+      `Absolute paths are not allowed in archive entries: ${entryPath}`,
+      "ABSOLUTE_PATH",
+      entryPath,
+    );
+  }
+
+  // Check for Windows drive letters even in relative-looking paths
+  if (/^[a-zA-Z]:/.test(normalizedEntryPath)) {
+    throw new PathTraversalError(
+      `Windows drive letters are not allowed in archive entries: ${entryPath}`,
+      "ABSOLUTE_PATH",
+      entryPath,
+    );
+  }
+
+  // Explicit check for ".." traversal patterns before any normalization
+  if (/(?:^|[\\/])\.\.(?:[\\/]|$)/.test(normalizedEntryPath)) {
+    throw new PathTraversalError(
+      `Path contains parent directory traversal: ${entryPath}`,
+      "TRAVERSAL",
+      entryPath,
+    );
+  }
+
+  // Check for Windows reserved names (CON, PRN, AUX, NUL, COM1-9, LPT1-9)
+  const pathSegments = normalizedEntryPath.split(path.sep).filter(Boolean);
+  for (const segment of pathSegments) {
+    if (/^(con|prn|aux|nul|com[1-9]|lpt[1-9])(\..*)?$/i.test(segment)) {
+      throw new PathTraversalError(
+        `Path contains Windows reserved name: ${segment}`,
+        "RESERVED_NAME",
+        entryPath,
+      );
+    }
+  }
+
+  // Layer 3: Post-Resolution Validation - Verify final containment
+  const resolvedDestDir = path.resolve(destDir);
+  const resolvedTargetPath = path.resolve(resolvedDestDir, normalizedEntryPath);
+
+  // Use path.relative() to determine if target is within destination
+  const relativePath = path.relative(resolvedDestDir, resolvedTargetPath);
+
+  // If the relative path starts with ".." or is absolute, the target escapes
+  if (relativePath.startsWith("..") || path.isAbsolute(relativePath)) {
+    throw new PathTraversalError(
+      `Resolved path escapes destination directory: ${entryPath}`,
+      "ESCAPE",
+      entryPath,
+    );
+  }
+
+  // Additional safety: ensure the resolved path starts with destination directory
+  // Handle proper separator to prevent sibling directory attacks
+  const destDirWithSep = resolvedDestDir.endsWith(path.sep)
+    ? resolvedDestDir
+    : resolvedDestDir + path.sep;
+
+  const targetPathForComparison = resolvedTargetPath + path.sep;
+
+  // Use case-insensitive comparison on Windows
+  const normalizedDest =
+    process.platform === "win32" ? destDirWithSep.toLowerCase() : destDirWithSep;
+  const normalizedTarget =
+    process.platform === "win32" ? targetPathForComparison.toLowerCase() : targetPathForComparison;
+
+  if (!normalizedTarget.startsWith(normalizedDest)) {
+    throw new PathTraversalError(
+      `Path escapes destination after normalization: ${entryPath}`,
+      "ESCAPE",
+      entryPath,
+    );
+  }
+
+  return resolvedTargetPath;
+}
+
 export type ArchiveKind = "tar" | "zip";
 
 export type ArchiveLogger = {
@@ -76,22 +206,21 @@ async function extractZip(params: { archivePath: string; destDir: string }): Pro
 
   for (const entry of entries) {
     const entryPath = entry.name.replaceAll("\\", "/");
+
+    // Validate the entry path using our secure validation function
+    // This will throw PathTraversalError if the path is unsafe
+    const safePath = validateZipEntryPath(entryPath, params.destDir);
+
     if (!entryPath || entryPath.endsWith("/")) {
-      const dirPath = path.resolve(params.destDir, entryPath);
-      if (!dirPath.startsWith(params.destDir)) {
-        throw new Error(`zip entry escapes destination: ${entry.name}`);
-      }
-      await fs.mkdir(dirPath, { recursive: true });
+      // Directory entry - create the directory structure
+      await fs.mkdir(safePath, { recursive: true });
       continue;
     }
 
-    const outPath = path.resolve(params.destDir, entryPath);
-    if (!outPath.startsWith(params.destDir)) {
-      throw new Error(`zip entry escapes destination: ${entry.name}`);
-    }
-    await fs.mkdir(path.dirname(outPath), { recursive: true });
+    // File entry - ensure parent directory exists and extract the file
+    await fs.mkdir(path.dirname(safePath), { recursive: true });
     const data = await entry.async("nodebuffer");
-    await fs.writeFile(outPath, data);
+    await fs.writeFile(safePath, data);
   }
 }
 
@@ -108,8 +237,20 @@ export async function extractArchive(params: {
 
   const label = kind === "zip" ? "extract zip" : "extract tar";
   if (kind === "tar") {
+    // For tar files, use secure extraction options to prevent path traversal
     await withTimeout(
-      tar.x({ file: params.archivePath, cwd: params.destDir }),
+      tar.x({
+        file: params.archivePath,
+        cwd: params.destDir,
+        // Enable security options to prevent path traversal
+        strict: true,
+        onEntry: (entry) => {
+          // Additional validation for tar entries
+          if (entry.path.includes("..") || path.isAbsolute(entry.path)) {
+            throw new Error(`Unsafe tar entry path: ${entry.path}`);
+          }
+        },
+      }),
       params.timeoutMs,
       label,
     );