From 5e4dc173f72b3159270c4864368b2e74ac1b34b2 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Tue, 9 Aug 2022 18:04:22 +0200
Subject: [PATCH 1/2] backend-common: validate zip archive paths when unpacking
 into a dir

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 .changeset/dirty-pears-allow.md                              | 5 +++++
 .../backend-common/src/reading/tree/ZipArchiveResponse.ts    | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/dirty-pears-allow.md

diff --git a/.changeset/dirty-pears-allow.md b/.changeset/dirty-pears-allow.md
new file mode 100644
index 0000000000..4b488ad79d
--- /dev/null
+++ b/.changeset/dirty-pears-allow.md
@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Added a second validation to the `dir()` method of ZIP archive responses returned from `readTree()` that ensures that extracted files do not fall outside the target directory.
diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
index 1a6658b0c2..4100af3d57 100644
--- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
+++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
@@ -25,6 +25,7 @@ import {
   ReadTreeResponseFile,
 } from '../types';
 import { streamToBuffer } from './util';
+import { resolveSafeChildPath } from '../../paths';
 
 /**
  * Wraps a zip archive stream into a tree response reader.
@@ -187,10 +188,10 @@ export class ZipArchiveResponse implements ReadTreeResponse {
       const dirname = platformPath.dirname(entryPath);
 
       if (dirname) {
-        await fs.mkdirp(platformPath.join(dir, dirname));
+        await fs.mkdirp(resolveSafeChildPath(dir, dirname));
       }
       return new Promise(async (resolve, reject) => {
-        const file = fs.createWriteStream(platformPath.join(dir, entryPath));
+        const file = fs.createWriteStream(resolveSafeChildPath(dir, entryPath));
         file.on('finish', resolve);
 
         content.on('error', reject);

From cb24f975aaf46f3bd526f3cd147aff3818a5a3fd Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Tue, 9 Aug 2022 18:27:13 +0200
Subject: [PATCH 2/2] backend-common: added test to ensure malicious zip
 archives are handled

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 .../src/reading/__fixtures__/mallory.zip      | Bin 0 -> 147 bytes
 .../reading/tree/ZipArchiveResponse.test.ts   |  22 ++++++++++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 packages/backend-common/src/reading/__fixtures__/mallory.zip

diff --git a/packages/backend-common/src/reading/__fixtures__/mallory.zip b/packages/backend-common/src/reading/__fixtures__/mallory.zip
new file mode 100644
index 0000000000000000000000000000000000000000..e45bd798b3748702af1f53a4780b335844dae9dc
GIT binary patch
literal 147
zcmWIWW@Zs#-~d9)M$S+MB)|=1>*?tiXQrg;l~j~yd+O?4^gVsXla(RBo1Fus5TpVG
l0=yZSbQw@hfyu#Wh)NKNWM+UjD;r3N5eO}Tv<ryC004W+5ibA$

literal 0
HcmV?d00001

diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts
index f1915cf99b..427c237802 100644
--- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts
+++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts
@@ -28,6 +28,9 @@ const archiveDataCorrupted = fs.readFileSync(
 const archiveDataWithExtraDir = fs.readFileSync(
   resolvePath(__filename, '../../__fixtures__/mock-with-extra-root-dir.zip'),
 );
+const archiveWithMaliciousEntry = fs.readFileSync(
+  resolvePath(__filename, '../../__fixtures__/mallory.zip'),
+);
 
 describe('ZipArchiveResponse', () => {
   beforeEach(() => {
@@ -35,6 +38,7 @@ describe('ZipArchiveResponse', () => {
       '/test-archive.zip': archiveData,
       '/test-archive-with-extra-root-dir.zip': archiveDataWithExtraDir,
       '/test-archive-corrupted.zip': archiveDataCorrupted,
+      '/test-archive-malicious.zip': archiveWithMaliciousEntry,
       '/tmp': mockFs.directory(),
     });
   });
@@ -167,4 +171,22 @@ describe('ZipArchiveResponse', () => {
       'invalid comment length. expected: 55. found: 0',
     );
   });
+
+  it('should throw on entries with a path outside the destination dir', async () => {
+    const stream = fs.createReadStream('/test-archive-malicious.zip');
+
+    const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag');
+    await expect(res.files()).rejects.toThrow(
+      'invalid relative path: ../side.txt',
+    );
+  });
+
+  it('should throw on entries that attempt to write outside destination dir', async () => {
+    const stream = fs.createReadStream('/test-archive-malicious.zip');
+
+    const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag');
+    await expect(res.dir()).rejects.toThrow(
+      'invalid relative path: ../side.txt',
+    );
+  });
 });