From 9d88abdb9006527bd7d1e3dea5443646af954875 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Fri, 2 Aug 2019 14:11:10 -0700 Subject: [PATCH] Update improvements --- ts/test/updater/common_test.ts | 52 ++++++++++++++++++++++++++- ts/updater/common.ts | 33 ++++++++++++++--- ts/updater/macos.ts | 66 +++++++++++++++++++++++++--------- ts/updater/windows.ts | 4 +-- 4 files changed, 132 insertions(+), 23 deletions(-) diff --git a/ts/test/updater/common_test.ts b/ts/test/updater/common_test.ts index 349c14fc8bfc..57d084606402 100644 --- a/ts/test/updater/common_test.ts +++ b/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'); + }); + }); + }); }); diff --git a/ts/updater/common.ts b/ts/updater/common.ts index 80cc8968367f..d044d113bd5f 100644 --- a/ts/updater/common.ts +++ b/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 { diff --git a/ts/updater/macos.ts b/ts/updater/macos.ts index 0ede2fe63369..8fde6f661022 100644 --- a/ts/updater/macos.ts +++ b/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 { 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, diff --git a/ts/updater/windows.ts b/ts/updater/windows.ts index 1035cf040c6a..82c5f086bf61 100644 --- a/ts/updater/windows.ts +++ b/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}')`