From fa4b2d412fc152249d8d328cf63007eb1f7343a5 Mon Sep 17 00:00:00 2001 From: Disconnect3d Date: Tue, 16 Jul 2019 22:28:16 +0200 Subject: [PATCH] Fix SUPPORTED_MEDIA_DOMAINS regex whitelist (#3459) The `SUPPORTED_MEDIA_DOMAINS` regex whitelist, used to check if media link comes from trusted hosts is invalid. It does not expose a security risk or I couldn't find an example for such as of now, but if someone would add a subdomain host to it using the same pattern, it would. A counter example below: ```js const SUPPORTED_MEDIA_DOMAINS = /^([^.]+\.)*(ytimg.com|cdninstagram.com|redd.it|imgur.com|fbcdn.net|pinimg.com)$/i; console.log('Testing redd.it: ' + SUPPORTED_MEDIA_DOMAINS.test('redd.it')); console.log('Testing reddjit: ' + SUPPORTED_MEDIA_DOMAINS.test('reddjit')); ``` Output: ``` $ node example.js Testing redd.it: true Testing reddjit: true ``` --- To be more clear, if someone would extend the regex in the future with e.g. `media.redd.it`, an attacker would be able to create a `mediaXredd.it` domain and bypass the whitelist. --- A visualisation of the incorrect regex can be found on https://regexper.com/#%5E%28%5B%5E.%5D%2B%5C.%29*%28ytimg.com%7Ccdninstagram.com%7Credd.it%7Cimgur.com%7Cfbcdn.net%7Cpinimg.com%29%24 The issue has been found with LGTM: https://lgtm.com/projects/g/signalapp/Signal-Desktop/snapshot/b626ef0b64bfa9867daff876a7cc680bc236897c/files/js/modules/link_previews.js?sort=name&dir=ASC&mode=heatmap#xdabadfc2bf20f0c3:1 --- js/modules/link_previews.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/modules/link_previews.js b/js/modules/link_previews.js index 07246a6ad3..1ddc000c35 100644 --- a/js/modules/link_previews.js +++ b/js/modules/link_previews.js @@ -68,7 +68,7 @@ function isStickerPack(link) { return (link || '').startsWith('https://signal.org/addstickers/'); } -const SUPPORTED_MEDIA_DOMAINS = /^([^.]+\.)*(ytimg.com|cdninstagram.com|redd.it|imgur.com|fbcdn.net|pinimg.com)$/i; +const SUPPORTED_MEDIA_DOMAINS = /^([^.]+\.)*(ytimg\.com|cdninstagram\.com|redd\.it|imgur\.com|fbcdn\.net|pinimg\.com)$/i; function isMediaLinkInWhitelist(link) { try { const url = new URL(link);