From 378dd913aa8d0fd0da29b0ffced8884579598b0f Mon Sep 17 00:00:00 2001
From: Daniel Lockyer <hi@daniellockyer.com>
Date: Fri, 7 Apr 2023 09:39:41 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20Fixed=20path=20traversal=20issue?=
 =?UTF-8?q?=20in=20theme=20files?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

refs https://github.com/TryGhost/Team/issues/2843

- Using encoded path traversal characters in URL's path allowed to fetch
  any file within active theme's folder, which is disallowed
- credits to: fuomag9 https://kiwi.fuo.fi/@fuomag9
---
 .../frontend/web/middleware/static-theme.js   | 36 +++++++++++++++++--
 .../web/middleware/static-theme.test.js       | 24 +++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/ghost/core/core/frontend/web/middleware/static-theme.js b/ghost/core/core/frontend/web/middleware/static-theme.js
index 4066c92a1d..e2a6d393fa 100644
--- a/ghost/core/core/frontend/web/middleware/static-theme.js
+++ b/ghost/core/core/frontend/web/middleware/static-theme.js
@@ -14,15 +14,45 @@ function isDeniedFile(file) {
     return deniedFiles.includes(base) || deniedFileTypes.includes(ext);
 }
 
+/**
+ * Copy from:
+ * https://github.com/pillarjs/send/blob/b69cbb3dc4c09c37917d08a4c13fcd1bac97ade5/index.js#L987-L1003
+ *
+ * Allows V8 to only deoptimize this fn instead of all
+ * of send().
+ *
+ * @param {string} filePath
+ * @returns {string|number} returns -1 number if decode decodeURIComponent throws
+ */
+function decode(filePath) {
+    try {
+        return decodeURIComponent(filePath);
+    } catch (err) {
+        return -1;
+    }
+}
+
+/**
+ *
+ * @param {string} file path to a requested file
+ * @returns {boolean}
+ */
 function isAllowedFile(file) {
+    const decodedFilePath = decode(file);
+    if (decodedFilePath === -1) {
+        return false;
+    }
+
+    const normalizedFilePath = path.normalize(decodedFilePath);
+
     const allowedFiles = ['manifest.json'];
     const allowedPath = '/assets/';
     const alwaysDeny = ['.hbs'];
 
-    const ext = path.extname(file);
-    const base = path.basename(file);
+    const ext = path.extname(normalizedFilePath);
+    const base = path.basename(normalizedFilePath);
 
-    return allowedFiles.includes(base) || (file.startsWith(allowedPath) && !alwaysDeny.includes(ext));
+    return allowedFiles.includes(base) || (normalizedFilePath.startsWith(allowedPath) && !alwaysDeny.includes(ext));
 }
 
 function forwardToExpressStatic(req, res, next) {
diff --git a/ghost/core/test/unit/frontend/web/middleware/static-theme.test.js b/ghost/core/test/unit/frontend/web/middleware/static-theme.test.js
index 24e7303c7c..7fabf13b1e 100644
--- a/ghost/core/test/unit/frontend/web/middleware/static-theme.test.js
+++ b/ghost/core/test/unit/frontend/web/middleware/static-theme.test.js
@@ -184,4 +184,28 @@ describe('staticTheme', function () {
             done();
         });
     });
+
+    it('should disallow path traversal', function (done) {
+        req.path = '/assets/built%2F..%2F..%2F/package.json';
+        req.method = 'GET';
+
+        staticTheme()(req, res, function next() {
+            activeThemeStub.called.should.be.false();
+            expressStaticStub.called.should.be.false();
+
+            done();
+        });
+    });
+
+    it('should not crash when malformatted URL sequence is passed', function (done) {
+        req.path = '/assets/built%2F..%2F..%2F%E0%A4%A/package.json';
+        req.method = 'GET';
+
+        staticTheme()(req, res, function next() {
+            activeThemeStub.called.should.be.false();
+            expressStaticStub.called.should.be.false();
+
+            done();
+        });
+    });
 });