fix: allow ../ symlink Streams when they're still within the package (#363)

* fix: allow symlinks that point to higher directories but still validate that they are within the filesystem

* add invalid symlink to test readstream failure on outside package

* add additional test for bad symlink fixture

Author: mmaietta

src/asar.ts

@@ -284,7 +284,13 @@ export async function createPackageFromStreams(dest: string, streams: AsarStream
           mode: stream.stat.mode,
           unpack: stream.unpacked,
         });
-        filesystem.insertLink(filename, stream.unpacked, stream.symlink);
+        filesystem.insertLink(
+          filename,
+          stream.unpacked,
+          path.dirname(filename),
+          stream.symlink,
+          src,
+        );
         break;
     }
     return Promise.resolve();

src/filesystem.ts

@@ -156,7 +156,14 @@ export class Filesystem {
     this.offset += BigInt(size);
   }
 
-  insertLink(p: string, shouldUnpack: boolean, link: string = this.resolveLink(p)) {
+  insertLink(
+    p: string,
+    shouldUnpack: boolean,
+    parentPath: string = fs.realpathSync(path.dirname(p)),
+    symlink: string = fs.readlinkSync(p), // /var/tmp => /private/var
+    src: string = fs.realpathSync(this.src),
+  ) {
+    const link = this.resolveLink(src, parentPath, symlink);
     if (link.startsWith('..')) {
       throw new Error(`${p}: file "${link}" links out of the package`);
     }
@@ -169,11 +176,9 @@ export class Filesystem {
     return link;
   }
 
-  private resolveLink(p: string) {
-    const symlink = fs.readlinkSync(p);
-    // /var/tmp => /private/var
-    const parentPath = fs.realpathSync(path.dirname(p));
-    const link = path.relative(fs.realpathSync(this.src), path.join(parentPath, symlink));
+  private resolveLink(src: string, parentPath: string, symlink: string) {
+    const target = path.join(parentPath, symlink);
+    const link = path.relative(src, target);
     return link;
   }
 

test/__snapshots__/api-spec.js.snap

@@ -497,6 +497,54 @@ Array [
 ]
 `;
 
+exports[`api should create package from array of NodeJS.ReadableStreams with valid symlinks 1`] = `
+Object {
+  "files": Object {
+    "A": Object {
+      "files": Object {
+        "real.txt": Object {
+          "integrity": Object {
+            "algorithm": "SHA256",
+            "blockSize": 4194304,
+            "blocks": Array [
+              "9ef260904c38a0173137ba5d9e95d1739b8dcac205847f202f6cff418a989bbe",
+            ],
+            "hash": "9ef260904c38a0173137ba5d9e95d1739b8dcac205847f202f6cff418a989bbe",
+          },
+          "offset": "0",
+          "size": 19,
+        },
+        "reverse-symlink.txt": Object {
+          "link": "B/reverse-symlink.txt",
+        },
+      },
+    },
+    "B": Object {
+      "files": Object {
+        "reverse-symlink.txt": Object {
+          "integrity": Object {
+            "algorithm": "SHA256",
+            "blockSize": 4194304,
+            "blocks": Array [
+              "766653f7a7a3e98a498888d5b7784bf808c5305bf5a01953ca866f7f36a9e111",
+            ],
+            "hash": "766653f7a7a3e98a498888d5b7784bf808c5305bf5a01953ca866f7f36a9e111",
+          },
+          "offset": "19",
+          "size": 22,
+        },
+      },
+    },
+    "Current": Object {
+      "link": "A",
+    },
+    "real.txt": Object {
+      "link": "Current/real.txt",
+    },
+  },
+}
+`;
+
 exports[`api should handle multibyte characters in paths 1`] = `
 Object {
   "files": Object {

test/api-spec.js

@@ -12,9 +12,7 @@ const { compFiles, isSymbolicLinkSync } = require('./util/compareFiles');
 const transform = require('./util/transformStream');
 const { TEST_APPS_DIR } = require('./util/constants');
 const { verifySmartUnpack } = require('./util/verifySmartUnpack');
-const { crawl, determineFileType } = require('../lib/crawlfs');
-const path = require('path');
-const walk = require('./util/walk');
+const createReadStreams = require('./util/createReadstreams');
 
 async function assertPackageListEquals(actualList, expectedFilename) {
   const expected = await fs.readFile(expectedFilename, 'utf8');
@@ -155,6 +153,13 @@ describe('api', function () {
         asar.extractAll('test/input/bad-symlink.asar', 'tmp/bad-symlink/');
       });
     });
+    it('should throw when packaging symlink outside package', async function () {
+      const src = 'test/input/packthis-with-bad-symlink/';
+      const out = 'tmp/packthis-read-stream-bad-symlink.asar';
+      assert.rejects(async () => {
+        await asar.createPackage(src, out);
+      });
+    });
   }
   it('should handle multibyte characters in paths', async () => {
     await asar.createPackageWithOptions(
@@ -171,29 +176,38 @@ describe('api', function () {
   });
   it('should create package from array of NodeJS.ReadableStreams', async () => {
     const src = 'test/input/packthis-glob/';
-    const filenames = walk(src);
+    const streams = await createReadStreams(src);
 
-    const streams = await Promise.all(
-      filenames.map(async (filename, i) => {
-        const meta = await determineFileType(filename);
-        return {
-          path: path.relative(src, filename),
-          streamGenerator:
-            meta.type === 'directory' ? undefined : () => fs.createReadStream(filename),
-          symlink: meta.type === 'link' ? await fs.readlink(filename) : undefined,
-          unpacked: filename.includes('x1'),
-          type: meta.type,
-          stat: meta.stat,
-        };
-      }),
-    );
-
-    await asar.createPackageFromStreams('tmp/packthis-read-stream.asar', streams);
-    await verifySmartUnpack('tmp/packthis-read-stream.asar');
-    await compFiles('tmp/packthis-read-stream.asar', 'test/expected/packthis-read-stream.asar');
-    asar.extractAll('tmp/packthis-read-stream.asar', 'tmp/extractthis-read-stream/');
+    const out = 'tmp/packthis-read-stream.asar';
+    await asar.createPackageFromStreams(out, streams);
+    await verifySmartUnpack(out);
+    await compFiles(out, 'test/expected/packthis-read-stream.asar');
+    asar.extractAll(out, 'tmp/extractthis-read-stream/');
     return compDirs('tmp/extractthis-read-stream/', src);
   });
+
+  it('should create package from array of NodeJS.ReadableStreams with valid symlinks', async function () {
+    if (os.platform() === 'win32') {
+      this.skip();
+    }
+    const src = 'test/input/packthis-with-symlink/';
+    const streams = await createReadStreams(src);
+
+    const out = 'tmp/packthis-read-stream-symlink.asar';
+    await asar.createPackageFromStreams(out, streams);
+    await verifySmartUnpack(out);
+    await compFiles(out, 'test/expected/packthis-read-stream-symlink.asar');
+    asar.extractAll(out, 'tmp/extractthis-read-stream-symlink/');
+    return compDirs('tmp/extractthis-read-stream-symlink/', src);
+  });
+  it('should throw when using NodeJS.ReadableStreams with symlink outside package', async function () {
+    const src = 'test/input/packthis-with-bad-symlink/';
+    const streams = await createReadStreams(src);
+
+    assert.rejects(async () => {
+      await asar.createPackageFromStreams(out, streams);
+    });
+  });
   it('should extract a text file from archive with multibyte characters in path', async () => {
     const actual = asar
       .extractFile('test/expected/packthis-unicode-path.asar', 'dir1/女の子.txt')

test/expected/packthis-read-stream-symlink.asar

@@ -0,0 +1 @@
+../../../reverse-symlink.txt

test/input/packthis-with-bad-symlink/A/reverse-symlink-outside-package.txt

@@ -0,0 +1 @@
+../B/reverse-symlink.txt

test/input/packthis-with-symlink/A/reverse-symlink.txt

@@ -0,0 +1 @@
+I SYMLINK TO SUPER DIR

test/input/packthis-with-symlink/B/reverse-symlink.txt

@@ -0,0 +1,26 @@
+const { determineFileType } = require('../../lib/crawlfs');
+const walk = require('./walk');
+const path = require('path');
+const fs = require('../../lib/wrapped-fs').default;
+
+const collectReadStreams = async (src) => {
+  const filenames = walk(src);
+
+  const streams = await Promise.all(
+    filenames.map(async (filename) => {
+      const meta = await determineFileType(filename);
+      return {
+        path: path.relative(src, filename),
+        streamGenerator:
+          meta.type === 'directory' ? undefined : () => fs.createReadStream(filename),
+        symlink: meta.type === 'link' ? await fs.readlink(filename) : undefined,
+        unpacked: filename.includes('x1'),
+        type: meta.type,
+        stat: meta.stat,
+      };
+    }),
+  );
+  return streams;
+};
+
+module.exports = collectReadStreams;