From ba48f22ad2cda80fc34229a392903718ef719655 Mon Sep 17 00:00:00 2001 From: Ske Date: Fri, 10 Jul 2020 16:35:52 +0200 Subject: [PATCH] Break proxied links if the sender doesn't have embed permission --- PluralKit.Bot/Proxy/ProxyService.cs | 15 ++++++++++----- PluralKit.Bot/Utils/DiscordUtils.cs | 21 ++++++++++++++++++++- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/PluralKit.Bot/Proxy/ProxyService.cs b/PluralKit.Bot/Proxy/ProxyService.cs index cf684811..9b6512d2 100644 --- a/PluralKit.Bot/Proxy/ProxyService.cs +++ b/PluralKit.Bot/Proxy/ProxyService.cs @@ -57,11 +57,14 @@ namespace PluralKit.Bot if (!await CheckBotPermissionsOrError(message.Channel)) return false; if (!CheckProxyNameBoundsOrError(match.Member.ProxyName(ctx))) return false; - // Check if we can mention everyone/here - var allowEveryone = (message.Channel.PermissionsInSync(message.Author) & Permissions.MentionEveryone) != 0; + // Check if the sender account can mention everyone/here + embed links + // we need to "mirror" these permissions when proxying to prevent exploits + var senderPermissions = message.Channel.PermissionsInSync(message.Author); + var allowEveryone = (senderPermissions & Permissions.MentionEveryone) != 0; + var allowEmbeds = (senderPermissions & Permissions.EmbedLinks) != 0; // Everything's in order, we can execute the proxy! - await ExecuteProxy(conn, message, ctx, match, allowEveryone); + await ExecuteProxy(conn, message, ctx, match, allowEveryone, allowEmbeds); return true; } @@ -88,12 +91,14 @@ namespace PluralKit.Bot } private async Task ExecuteProxy(IPKConnection conn, DiscordMessage trigger, MessageContext ctx, - ProxyMatch match, bool allowEveryone) + ProxyMatch match, bool allowEveryone, bool allowEmbeds) { // Send the webhook + var content = match.ProxyContent; + if (!allowEmbeds) content = content.BreakLinkEmbeds(); var id = await _webhookExecutor.ExecuteWebhook(trigger.Channel, match.Member.ProxyName(ctx), match.Member.ProxyAvatar(ctx), - match.ProxyContent, trigger.Attachments, allowEveryone); + content, trigger.Attachments, allowEveryone); Task SaveMessage() => _data.AddMessage(conn, trigger.Author.Id, trigger.Channel.GuildId, trigger.Channel.Id, id, trigger.Id, match.Member.Id); Task LogMessage() => _logChannel.LogMessage(ctx, match, trigger, id).AsTask(); diff --git a/PluralKit.Bot/Utils/DiscordUtils.cs b/PluralKit.Bot/Utils/DiscordUtils.cs index 84c02bdb..2b9d2959 100644 --- a/PluralKit.Bot/Utils/DiscordUtils.cs +++ b/PluralKit.Bot/Utils/DiscordUtils.cs @@ -29,6 +29,14 @@ namespace PluralKit.Bot private static readonly Regex USER_MENTION = new Regex("<@!?(\\d{17,19})>"); private static readonly Regex ROLE_MENTION = new Regex("<@&(\\d{17,19})>"); private static readonly Regex EVERYONE_HERE_MENTION = new Regex("@(everyone|here)"); + + // Discord uses Khan Academy's simple-markdown library for parsing Markdown, + // which uses the following regex for link detection: + // ^(https?:\/\/[^\s<]+[^<.,:;"')\]\s]) + // Source: https://raw.githubusercontent.com/DJScias/Discord-Datamining/master/2020/2020-07-10/47efb8681861cb7c5ffa.js @ line 20633 + // corresponding to: https://github.com/Khan/simple-markdown/blob/master/src/index.js#L1489 + // I added ? at the start/end; they need to be handled specially later... + private static readonly Regex UNBROKEN_LINK_REGEX = new Regex("?"); private static readonly FieldInfo _roleIdsField = typeof(DiscordMember).GetField("_role_ids", BindingFlags.NonPublic | BindingFlags.Instance); @@ -197,7 +205,7 @@ namespace PluralKit.Bot public static string EscapeBacktickPair(this string input){ Regex doubleBacktick = new Regex(@"``", RegexOptions.Multiline); //Run twice to catch any pairs that are created from the first pass, pairs shouldn't be created in the second as they are created from odd numbers of backticks, even numbers are all caught on the first pass - if(input != null) return doubleBacktick.Replace(doubleBacktick.Replace(input, @"`‌`"),@"`‌`"); + if(input != null) return doubleBacktick.Replace(doubleBacktick.Replace(input, @"`‌ `"),@"`‌`"); else return input; } @@ -277,5 +285,16 @@ namespace PluralKit.Bot return eb; } + + public static string BreakLinkEmbeds(this string str) => + // Encases URLs in + UNBROKEN_LINK_REGEX.Replace(str, match => + { + // Don't break already-broken links + // The regex will include the brackets in the match, so we can check for their presence here + if (match.Value.StartsWith("<") && match.Value.EndsWith(">")) + return match.Value; + return $"<{match.Value}>"; + }); } } \ No newline at end of file