From aaed0db2e5d77ae636785dbca5332952d88636e3 Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Wed, 26 Aug 2020 17:16:59 -0500 Subject: [PATCH] Verify sticker data in getDataFromLink --- js/modules/stickers.d.ts | 2 + js/modules/stickers.js | 43 ++++++++--- js/views/conversation_view.js | 8 +- test/stickers_test.js | 136 ++++++++++++++++++++++++++++++++++ ts/util/lint/exceptions.json | 2 +- 5 files changed, 178 insertions(+), 13 deletions(-) diff --git a/js/modules/stickers.d.ts b/js/modules/stickers.d.ts index e864f0d8ff..e294095909 100644 --- a/js/modules/stickers.d.ts +++ b/js/modules/stickers.d.ts @@ -10,4 +10,6 @@ export function downloadStickerPack( } ): Promise; +export function isPackIdValid(packId: unknown): packId is string; + export function redactPackId(packId: string): string; diff --git a/js/modules/stickers.js b/js/modules/stickers.js index d4faac7266..b5f55985ba 100644 --- a/js/modules/stickers.js +++ b/js/modules/stickers.js @@ -5,7 +5,8 @@ navigator, reduxStore, reduxActions, - URL + URL, + URLSearchParams */ const BLESSED_PACKS = { @@ -27,10 +28,11 @@ const BLESSED_PACKS = { }, }; +const VALID_PACK_ID_REGEXP = /^[0-9a-f]{32}$/i; + const { isNumber, pick, reject, groupBy, values } = require('lodash'); const pMap = require('p-map'); const Queue = require('p-queue').default; -const qs = require('qs'); const { makeLookup } = require('../../ts/util/makeLookup'); const { @@ -65,6 +67,7 @@ module.exports = { load, maybeDeletePack, downloadQueuedPacks, + isPackIdValid, redactPackId, removeEphemeralPack, savePackMetadata, @@ -90,18 +93,36 @@ async function load() { } function getDataFromLink(link) { - const { hash } = new URL(link); + let url; + try { + url = new URL(link); + } catch (err) { + return null; + } + + const { hash } = url; if (!hash) { return null; } - const data = hash.slice(1); - const params = qs.parse(data); + let params; + try { + params = new URLSearchParams(hash.slice(1)); + } catch (err) { + return null; + } - return { - id: params.pack_id, - key: params.pack_key, - }; + const id = params.get('pack_id'); + if (!isPackIdValid(id)) { + return null; + } + + const key = params.get('pack_key'); + if (!key) { + return null; + } + + return { id, key }; } function getInstalledStickerPacks() { @@ -231,6 +252,10 @@ function getInitialState() { return initialState; } +function isPackIdValid(packId) { + return typeof packId === 'string' && VALID_PACK_ID_REGEXP.test(packId); +} + function redactPackId(packId) { return `[REDACTED]${packId.slice(-3)}`; } diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index b6183a2d41..aa6f9c2f3c 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -2963,11 +2963,13 @@ pack.status === 'downloaded' || pack.status === 'installed'); - let id; - let key; + const dataFromLink = window.Signal.Stickers.getDataFromLink(url); + if (!dataFromLink) { + return null; + } + const { id, key } = dataFromLink; try { - ({ id, key } = window.Signal.Stickers.getDataFromLink(url)); const keyBytes = window.Signal.Crypto.bytesFromHexString(key); const keyBase64 = window.Signal.Crypto.arrayBufferToBase64(keyBytes); diff --git a/test/stickers_test.js b/test/stickers_test.js index c289abf19f..1be0e8b4fa 100644 --- a/test/stickers_test.js +++ b/test/stickers_test.js @@ -3,6 +3,142 @@ const { Stickers } = Signal; describe('Stickers', () => { + describe('getDataFromLink', () => { + it('returns null for invalid URLs', () => { + assert.isNull(Stickers.getDataFromLink('https://')); + assert.isNull(Stickers.getDataFromLink('signal.art/addstickers/')); + }); + + it("returns null for URLs that don't have a hash", () => { + assert.isNull( + Stickers.getDataFromLink('https://signal.art/addstickers/') + ); + assert.isNull( + Stickers.getDataFromLink('https://signal.art/addstickers/#') + ); + }); + + it('returns null when no key or pack ID is found', () => { + assert.isNull( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id=c8c83285b547872ac4c589d64a6edd6a' + ) + ); + assert.isNull( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id=c8c83285b547872ac4c589d64a6edd6a&pack_key=' + ) + ); + assert.isNull( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_key=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e' + ) + ); + assert.isNull( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_key=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e&pack_id=' + ) + ); + }); + + it('returns null when the pack ID is invalid', () => { + assert.isNull( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id=garbage&pack_key=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e' + ) + ); + }); + + it('returns null if the ID or key are passed as arrays', () => { + assert.isNull( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id[]=c8c83285b547872ac4c589d64a6edd6a&pack_key=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e' + ) + ); + assert.isNull( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id=c8c83285b547872ac4c589d64a6edd6a&pack_key[]=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e' + ) + ); + }); + + it('parses the ID and key from the hash', () => { + assert.deepEqual( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id=c8c83285b547872ac4c589d64a6edd6a&pack_key=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e' + ), + { + id: 'c8c83285b547872ac4c589d64a6edd6a', + key: + '59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e', + } + ); + }); + + it('ignores additional hash parameters', () => { + assert.deepEqual( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id=c8c83285b547872ac4c589d64a6edd6a&pack_key=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e&pack_foo=bar' + ), + { + id: 'c8c83285b547872ac4c589d64a6edd6a', + key: + '59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e', + } + ); + }); + + it('only parses the first ID and key from the hash if more than one is supplied', () => { + assert.deepEqual( + Stickers.getDataFromLink( + 'https://signal.art/addstickers/#pack_id=c8c83285b547872ac4c589d64a6edd6a&pack_key=59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e&pack_id=extra&pack_key=extra' + ), + { + id: 'c8c83285b547872ac4c589d64a6edd6a', + key: + '59bb3a8860f0e6a5a83a5337a015c8d55ecd2193f82d77202f3b8112a845636e', + } + ); + }); + }); + + describe('isPackIdValid', () => { + it('returns false for non-strings', () => { + assert.isFalse(Stickers.isPackIdValid(undefined)); + assert.isFalse(Stickers.isPackIdValid(null)); + assert.isFalse(Stickers.isPackIdValid(123)); + assert.isFalse(Stickers.isPackIdValid(123)); + assert.isFalse( + Stickers.isPackIdValid(['b9439fa5fdc8b9873fe64f01b88b8ccf']) + ); + assert.isFalse( + // eslint-disable-next-line no-new-wrappers + Stickers.isPackIdValid(new String('b9439fa5fdc8b9873fe64f01b88b8ccf')) + ); + }); + + it('returns false for invalid pack IDs', () => { + assert.isFalse(Stickers.isPackIdValid('')); + assert.isFalse( + Stickers.isPackIdValid('x9439fa5fdc8b9873fe64f01b88b8ccf') + ); + assert.isFalse( + // This is one character too short. + Stickers.isPackIdValid('b9439fa5fdc8b9873fe64f01b88b8cc') + ); + assert.isFalse( + // This is one character too long. + Stickers.isPackIdValid('b9439fa5fdc8b9873fe64f01b88b8ccfa') + ); + }); + + it('returns true for valid pack IDs', () => { + assert.isTrue(Stickers.isPackIdValid('b9439fa5fdc8b9873fe64f01b88b8ccf')); + assert.isTrue(Stickers.isPackIdValid('3eff225a1036a58a7530b312dd92f8d8')); + assert.isTrue(Stickers.isPackIdValid('DDFD48B8097DA7A4E928192B10963F6A')); + }); + }); + describe('redactPackId', () => { it('redacts pack IDs', () => { assert.strictEqual( diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index 6873f62664..a52461bc0d 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -271,7 +271,7 @@ "rule": "jQuery-load(", "path": "js/modules/stickers.js", "line": "async function load() {", - "lineNumber": 77, + "lineNumber": 80, "reasonCategory": "falseMatch", "updated": "2019-04-26T17:48:30.675Z" },