Update improvements

ts/test/updater/common_test.ts

@@ -1,6 +1,12 @@
 import { assert } from 'chai';
 
-import { getUpdateFileName, getVersion } from '../../updater/common';
+import {
+  createTempDir,
+  getUpdateFileName,
+  getVersion,
+  isUpdateFileNameValid,
+  validatePath,
+} from '../../updater/common';
 
 describe('updater/signatures', () => {
   const windows = `version: 1.23.2
@@ -74,4 +80,48 @@ releaseDate: '2019-03-29T01:53:23.881Z'
       );
     });
   });
+
+  describe('#isUpdateFileNameValid', () => {
+    it('returns true for normal filenames', () => {
+      assert.strictEqual(
+        isUpdateFileNameValid('signal-desktop-win-1.23.2.exe'),
+        true
+      );
+      assert.strictEqual(
+        isUpdateFileNameValid('signal-desktop-mac-1.23.2-beta.1.zip'),
+        true
+      );
+    });
+    it('returns false for problematic names', () => {
+      assert.strictEqual(
+        isUpdateFileNameValid('../signal-desktop-win-1.23.2.exe'),
+        false
+      );
+      assert.strictEqual(
+        isUpdateFileNameValid('%signal-desktop-mac-1.23.2-beta.1.zip'),
+        false
+      );
+      assert.strictEqual(
+        isUpdateFileNameValid('@signal-desktop-mac-1.23.2-beta.1.zip'),
+        false
+      );
+    });
+  });
+
+  describe('#validatePath', () => {
+    it('succeeds for simple children', async () => {
+      const base = await createTempDir();
+      validatePath(base, `${base}/child`);
+      validatePath(base, `${base}/child/grandchild`);
+    });
+    it('returns false for problematic names', async () => {
+      const base = await createTempDir();
+      assert.throws(() => {
+        validatePath(base, `${base}/../child`);
+      });
+      assert.throws(() => {
+        validatePath(base, '/root');
+      });
+    });
+  });
 });

ts/updater/common.ts

@@ -3,7 +3,7 @@ import {
   statSync,
   writeFile as writeFileCallback,
 } from 'fs';
-import { join } from 'path';
+import { join, normalize } from 'path';
 import { tmpdir } from 'os';
 
 // @ts-ignore
@@ -80,6 +80,16 @@ export async function checkForUpdates(
   return null;
 }
 
+export function validatePath(basePath: string, targetPath: string) {
+  const normalized = normalize(targetPath);
+
+  if (!normalized.startsWith(basePath)) {
+    throw new Error(
+      `validatePath: Path ${normalized} is not under base path ${basePath}`
+    );
+  }
+}
+
 export async function downloadUpdate(
   fileName: string,
   logger: LoggerType
@@ -96,6 +106,9 @@ export async function downloadUpdate(
     const targetUpdatePath = join(tempDir, fileName);
     const targetSignaturePath = join(tempDir, getSignatureFileName(fileName));
 
+    validatePath(tempDir, targetUpdatePath);
+    validatePath(tempDir, targetSignaturePath);
+
     logger.info(`downloadUpdate: Downloading ${signatureUrl}`);
     const { body } = await get(signatureUrl, getGotOptions());
     await writeFile(targetSignaturePath, body);
@@ -228,14 +241,26 @@ export function getVersion(yaml: string): string | undefined {
   return;
 }
 
+const validFile = /^[A-Za-z0-9\.\-]+$/;
+export function isUpdateFileNameValid(name: string) {
+  return validFile.test(name);
+}
+
 export function getUpdateFileName(yaml: string) {
   const info = parseYaml(yaml);
 
-  if (info && info.path) {
-    return info.path;
+  if (!info || !info.path) {
+    throw new Error('getUpdateFileName: No path present in YAML file');
   }
 
-  return;
+  const path = info.path;
+  if (!isUpdateFileNameValid(path)) {
+    throw new Error(
+      `getUpdateFileName: Path '${path}' contains invalid characters`
+    );
+  }
+
+  return path;
 }
 
 function parseYaml(yaml: string): any {

ts/updater/macos.ts

@@ -7,6 +7,7 @@ import { v4 as getGuid } from 'uuid';
 import { app, autoUpdater, BrowserWindow, dialog } from 'electron';
 import { get as getFromConfig } from 'config';
 import { gt } from 'semver';
+import got from 'got';
 
 import {
   checkForUpdates,
@@ -79,7 +80,7 @@ async function checkDownloadAndInstall(
     }
 
     const publicKey = hexToBinary(getFromConfig('updatesPublicKey'));
-    const verified = verifySignature(updateFilePath, version, publicKey);
+    const verified = await verifySignature(updateFilePath, version, publicKey);
     if (!verified) {
       // Note: We don't delete the cache here, because we don't want to continually
       //   re-download the broken release. We will download it only once per launch.
@@ -148,6 +149,7 @@ async function handToAutoUpdate(
   logger: LoggerType
 ): Promise<void> {
   return new Promise((resolve, reject) => {
+    const token = getGuid();
     const updateFileUrl = generateFileUrl();
     const server = createServer();
     let serverUrl: string;
@@ -173,6 +175,12 @@ async function handToAutoUpdate(
           return;
         }
 
+        if (url === '/token') {
+          writeTokenResponse(token, response);
+
+          return;
+        }
+
         if (!url || !url.startsWith(updateFileUrl)) {
           write404(url, response, logger);
 
@@ -183,24 +191,37 @@ async function handToAutoUpdate(
       }
     );
 
-    server.listen(0, '127.0.0.1', () => {
-      serverUrl = getServerUrl(server);
+    server.listen(0, '127.0.0.1', async () => {
+      try {
+        serverUrl = getServerUrl(server);
 
-      autoUpdater.on('error', (error: Error) => {
-        logger.error('autoUpdater: error', getPrintableError(error));
+        autoUpdater.on('error', (error: Error) => {
+          logger.error('autoUpdater: error', getPrintableError(error));
+          reject(error);
+        });
+        autoUpdater.on('update-downloaded', () => {
+          logger.info('autoUpdater: update-downloaded event fired');
+          shutdown(server, logger);
+          resolve();
+        });
+
+        const response = await got.get(`${serverUrl}/token`);
+        if (JSON.parse(response.body).token !== token) {
+          throw new Error(
+            'autoUpdater: did not receive token back from updates server'
+          );
+        }
+
+        autoUpdater.setFeedURL({
+          url: serverUrl,
+          headers: { 'Cache-Control': 'no-cache' },
+        });
+        autoUpdater.checkForUpdates();
+      } catch (error) {
         reject(error);
-      });
-      autoUpdater.on('update-downloaded', () => {
-        logger.info('autoUpdater: update-downloaded event fired');
-        shutdown(server, logger);
-        resolve();
-      });
 
-      autoUpdater.setFeedURL({
-        url: serverUrl,
-        headers: { 'Cache-Control': 'no-cache' },
-      });
-      autoUpdater.checkForUpdates();
+        return;
+      }
     });
   });
 }
@@ -254,6 +275,19 @@ function writeJSONResponse(url: string, response: ServerResponse) {
   response.end(data);
 }
 
+function writeTokenResponse(token: string, response: ServerResponse) {
+  const data = Buffer.from(
+    JSON.stringify({
+      token,
+    })
+  );
+  response.writeHead(200, {
+    'Content-Type': 'application/json',
+    'Content-Length': data.byteLength,
+  });
+  response.end(data);
+}
+
 function write404(
   url: string | undefined,
   response: ServerResponse,

ts/updater/windows.ts

@@ -83,7 +83,7 @@ async function checkDownloadAndInstall(
     }
 
     const publicKey = hexToBinary(getFromConfig('updatesPublicKey'));
-    const verified = verifySignature(updateFilePath, version, publicKey);
+    const verified = await verifySignature(updateFilePath, version, publicKey);
     if (!verified) {
       // Note: We don't delete the cache here, because we don't want to continually
       //   re-download the broken release. We will download it only once per launch.
@@ -164,7 +164,7 @@ async function verifyAndInstall(
   logger: LoggerType
 ) {
   const publicKey = hexToBinary(getFromConfig('updatesPublicKey'));
-  const verified = verifySignature(updateFilePath, newVersion, publicKey);
+  const verified = await verifySignature(updateFilePath, newVersion, publicKey);
   if (!verified) {
     throw new Error(
       `Downloaded update did not pass signature verification (version: '${newVersion}'; fileName: '${fileName}')`