From 6df37dd7153e8f41cc1c4caf2c25a214d9797528 Mon Sep 17 00:00:00 2001
From: Matt Bishop <mbishop@bitwarden.com>
Date: Fri, 3 Feb 2023 14:24:49 -0500
Subject: [PATCH] [CSA-28] Use path normalization in API requests (#4580)

* Use path normalization in API requests

* Remove CLI webpack config change that's unneeded

* Add additional tests
---
 apps/browser/webpack.config.js          |  2 +-
 apps/web/webpack.config.js              |  2 +-
 libs/common/spec/misc/utils.spec.ts     | 20 ++++++++++++++++++++
 libs/common/src/misc/utils.ts           | 11 +++++++++++
 libs/common/src/services/api.service.ts |  5 +----
 5 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/apps/browser/webpack.config.js b/apps/browser/webpack.config.js
index 4d2f96d96a..d1adca877e 100644
--- a/apps/browser/webpack.config.js
+++ b/apps/browser/webpack.config.js
@@ -209,7 +209,7 @@ const mainConfig = {
       util: require.resolve("util/"),
       url: require.resolve("url/"),
       fs: false,
-      path: false,
+      path: require.resolve("path-browserify"),
     },
   },
   output: {
diff --git a/apps/web/webpack.config.js b/apps/web/webpack.config.js
index 42b22121ac..15abbe6f6e 100644
--- a/apps/web/webpack.config.js
+++ b/apps/web/webpack.config.js
@@ -355,9 +355,9 @@ const webpackConfig = {
       util: require.resolve("util/"),
       assert: false,
       url: false,
-      path: false,
       fs: false,
       process: false,
+      path: require.resolve("path-browserify"),
     },
   },
   output: {
diff --git a/libs/common/spec/misc/utils.spec.ts b/libs/common/spec/misc/utils.spec.ts
index e2fb27a1eb..84ce28f4b2 100644
--- a/libs/common/spec/misc/utils.spec.ts
+++ b/libs/common/spec/misc/utils.spec.ts
@@ -326,4 +326,24 @@ describe("Utils Service", () => {
       );
     });
   });
+
+  describe("normalizePath", () => {
+    it("removes a single traversal", () => {
+      expect(Utils.normalizePath("../test")).toBe("test");
+    });
+
+    it("removes deep traversals", () => {
+      expect(Utils.normalizePath("../../test")).toBe("test");
+    });
+
+    it("removes intermediate traversals", () => {
+      expect(Utils.normalizePath("test/../test")).toBe("test");
+    });
+
+    it("removes multiple encoded traversals", () => {
+      expect(
+        Utils.normalizePath("api/sends/access/..%2f..%2f..%2fapi%2fsends%2faccess%2fsendkey")
+      ).toBe("api/sends/access/sendkey");
+    });
+  });
 });
diff --git a/libs/common/src/misc/utils.ts b/libs/common/src/misc/utils.ts
index 821f02d0a0..9656852984 100644
--- a/libs/common/src/misc/utils.ts
+++ b/libs/common/src/misc/utils.ts
@@ -1,4 +1,6 @@
 /* eslint-disable no-useless-escape */
+import * as path from "path";
+
 import { getHostname, parse } from "tldts";
 import { Merge } from "type-fest";
 
@@ -498,6 +500,15 @@ export class Utils {
     );
   }
 
+  /**
+   * Normalizes a path for defense against attacks like traversals
+   * @param denormalizedPath
+   * @returns
+   */
+  static normalizePath(denormalizedPath: string): string {
+    return path.normalize(decodeURIComponent(denormalizedPath)).replace(/^(\.\.(\/|\\|$))+/, "");
+  }
+
   private static isMobile(win: Window) {
     let mobile = false;
     ((a) => {
diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts
index 3ee6ef585b..957183da04 100644
--- a/libs/common/src/services/api.service.ts
+++ b/libs/common/src/services/api.service.ts
@@ -1962,11 +1962,8 @@ export class ApiService implements ApiServiceAbstraction {
   ): Promise<any> {
     apiUrl = Utils.isNullOrWhitespace(apiUrl) ? this.environmentService.getApiUrl() : apiUrl;
 
-    const requestUrl = apiUrl + path;
     // Prevent directory traversal from malicious paths
-    if (new URL(requestUrl).href !== requestUrl) {
-      return Promise.reject("Invalid request url path.");
-    }
+    const requestUrl = apiUrl + Utils.normalizePath(path);
 
     const headers = new Headers({
       "Device-Type": this.deviceType,