Harden toFSPath against path traversal

whtree/jssdk/services/src/resources.ts

@@ -1,20 +1,66 @@
 import * as path from "node:path";
 import { backendConfig } from "./config";
 
-export function toFSPath(resource: string, options: { keepUnmatched: true }): string;
-export function toFSPath(resource: string, options: { allowUnmatched: true }): string | null;
-export function toFSPath(resource: string, options?: { allowUnmatched?: boolean; keepUnmatched?: boolean }): string;
+export type ToFSPathOptions = {
+  /** Do not fail if the path cannot be matched to a filesystem path */
+  allowUnmatched?: boolean;
+  /** Return the original path if unmatched. Implies allowUnmatched */
+  keepUnmatched?: boolean;
+  /** Resource path (eg `mod::mymodule/data/`) that the resolved path must stay inside. If set, '.' and '..'
+      segments are permitted in the resource path, as long as the result does not escape this root nor the
+      module/storage folder it was resolved against. The root itself may not contain '.' or '..' segments. */
+  root?: string;
+};
+
+export function toFSPath(resource: string, options: ToFSPathOptions & { keepUnmatched: true }): string;
+export function toFSPath(resource: string, options: ToFSPathOptions & { allowUnmatched: true }): string | null;
+export function toFSPath(resource: string, options?: ToFSPathOptions): string;
 
 /** Resolve a resource path to a filesystem path
     @param resource - Path to resolve
-    @param allowUnmatched - Do not fail if the path cannot be matched to a filesystem path
-    @param keepUnmatched - Return the original path if unmatched. Implies allowUnmatched
+    @param options - Options, see {@link ToFSPathOptions}
     @returns Absolute file system path. A succesful return does not imply the path actually exists
-    @throws If the path cannot be mapped to a filesystem path
+    @throws If the path cannot be mapped to a filesystem path, if it contains '.' or '..' segments (and no root is set),
+            or if it escapes the specified root or its module/storage folder
 */
-export function toFSPath(resource: string, { allowUnmatched = false, keepUnmatched = false } = {}) {
+export function toFSPath(resource: string, { allowUnmatched = false, keepUnmatched = false, root }: ToFSPathOptions = {}) {
+  const resolved = resolveFSBase(resource, { allowUnmatched, keepUnmatched });
+  if (typeof resolved !== "object" || resolved === null) //unmatched, return as-is (null or the original resource)
+    return resolved;
+
+  const { basedir, subpath } = resolved;
+  const segments = subpath.split("/");
+  const hasDotSegments = segments.some(segment => segment === "." || segment === "..");
+  if (hasDotSegments && root === undefined)
+    throw new Error(`Invalid resource path '${resource}': '.' and '..' path segments are not allowed`);
+
+  let result = path.join(basedir, subpath);
+  if (hasDotSegments && [".", ".."].includes(segments.at(-1)!) && !result.endsWith("/"))
+    result += "/"; //'x/..' refers to a directory, keep the trailing slash like we do for `mod::system`
+
+  if (!isInsideDir(result, basedir))
+    throw new Error(`Invalid resource path '${resource}': it escapes its base folder`);
+
+  if (root === undefined)
+    return result;
+
+  const rootpath = toFSPath(root); //throws if root itself is invalid or contains dot segments
+  if (!isInsideDir(result, rootpath))
+    throw new Error(`Invalid resource path '${resource}': it escapes the root '${root}'`);
+
+  return result;
+}
+
+/** Test whether a (normalized) path is equal to or inside a directory */
+function isInsideDir(testpath: string, dir: string) {
+  const dirWithSlash = dir.endsWith("/") ? dir : dir + "/";
+  return testpath === dirWithSlash || testpath === dirWithSlash.slice(0, -1) || testpath.startsWith(dirWithSlash);
+}
+
+/** Split a resource into the filesystem base directory it maps to and the remaining subpath */
+function resolveFSBase(resource: string, { allowUnmatched, keepUnmatched }: { allowUnmatched: boolean; keepUnmatched: boolean }): { basedir: string; subpath: string } | string | null {
   if (resource.startsWith("@webhare/"))
-    return backendConfig.installationRoot + "jssdk/" + resource.substring(9);
+    return { basedir: backendConfig.installationRoot + "jssdk/", subpath: resource.substring(9) };
 
   if (resource.startsWith("@mod-"))
     resource = `mod::${resource.substring(5)}`;
@@ -46,10 +92,8 @@ export function toFSPath(resource: string, { allowUnmatched = false, keepUnmatch
 
     const basedir = namespace === "mod" ? modinfo.root : `${backendConfig.dataRoot}storage/${modulename}/`;
 
-    if (nextslash === -1)
-      return basedir; //we'll always terminate a path like `mod::system` with a slash
-    else
-      return path.join(basedir, restpath.substring(nextslash));
+    //we'll always terminate a path like `mod::system` with a slash
+    return { basedir, subpath: nextslash === -1 ? "" : restpath.substring(nextslash + 1) };
   }
 
   if (keepUnmatched)

whtree/modules/webhare_testsuite/tests/system/nodejs/test_resources.ts

@@ -103,6 +103,33 @@ async function testPaths() {
   test.eq(services.backendConfig.dataRoot + "storage/system/xyz/", services.toFSPath("storage::system/xyz/"));
   test.eq(services.backendConfig.dataRoot + "storage/system/", services.toFSPath("storage::system"));
 
+  //Path traversal: '.' and '..' segments are not allowed without a root
+  const sysroot = services.backendConfig.module.system.root;
+  test.throws(/not allowed/, () => services.toFSPath("mod::system/../webhare_testsuite/"));
+  test.throws(/not allowed/, () => services.toFSPath("mod::system/lib/../../"));
+  test.throws(/not allowed/, () => services.toFSPath("mod::system/./lib/"));
+  test.throws(/not allowed/, () => services.toFSPath("mod::system/lib/.."));
+  test.throws(/not allowed/, () => services.toFSPath("@mod-system/../x"));
+  test.throws(/not allowed/, () => services.toFSPath("storage::system/../x"));
+  test.throws(/not allowed/, () => services.toFSPath("@webhare/../modules/"));
+  test.throws(/not allowed/, () => services.toFSPath("mod::system/../x", { allowUnmatched: true }));
+  test.eq(sysroot + "lib/..x/.y", services.toFSPath("mod::system/lib/..x/.y")); //not dot segments
+
+  //With a root, dot segments are allowed as long as we stay inside both the root and the module
+  test.eq(sysroot + "lib/database.whlib", services.toFSPath("mod::system/lib/./x/../database.whlib", { root: "mod::system/" }));
+  test.eq(sysroot + "lib/", services.toFSPath("mod::system/lib/x/..", { root: "mod::system/lib/" }));
+  test.eq(sysroot + "lib/", services.toFSPath("mod::system/lib/x/..", { root: "mod::system/lib" }));
+  test.eq(sysroot + "lib/database.whlib", services.toFSPath("mod::system/lib/database.whlib", { root: "mod::system/lib/" }));
+  test.eq(sysroot, services.toFSPath("mod::system/lib/..", { root: "mod::system" }));
+  test.throws(/escapes the root/, () => services.toFSPath("mod::system/lib/../js/", { root: "mod::system/lib/" }));
+  test.throws(/escapes the root/, () => services.toFSPath("mod::system/lib/../lib2/", { root: "mod::system/lib/" }));
+  test.throws(/escapes the root/, () => services.toFSPath("mod::system/js/", { root: "mod::system/lib/" }));
+  test.throws(/escapes the root/, () => services.toFSPath("mod::webhare_testsuite/", { root: "mod::system/" }));
+  test.throws(/escapes its base folder/, () => services.toFSPath("mod::system/../webhare_testsuite/", { root: "mod::system/" }));
+  test.throws(/escapes its base folder/, () => services.toFSPath("storage::system/../../", { root: "storage::system/" }));
+  test.throws(/not allowed/, () => services.toFSPath("mod::system/lib/", { root: "mod::system/lib/../" }));
+  test.throws(/No such resource/, () => services.toFSPath("mod::system/lib/", { root: "mod::nosuchmodule/" }));
+
   test.eq(/^https?:.*/, services.backendConfig.backendURL);
 
   const systempath = services.backendConfig.module.system.root;