From 48b327be3e0e8cf8ddf47b8155cc2c06c1bcd336 Mon Sep 17 00:00:00 2001 From: Chris Johnston Date: Sat, 16 Mar 2019 11:34:50 -0700 Subject: [PATCH] fix: fix false invalidation when decoding token User Ids (#1278) * add a util method for padding base64 strings if they are not of an expected length * return the original string if it already contains padding, do not throw * add tests for padding method, and for token that needs padding --- src/Discord.Net.Core/Utils/TokenUtils.cs | 45 +++++++++++++++++++++- test/Discord.Net.Tests/Tests.TokenUtils.cs | 38 ++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/Discord.Net.Core/Utils/TokenUtils.cs b/src/Discord.Net.Core/Utils/TokenUtils.cs index 68aad5d96..b52ba3dd6 100644 --- a/src/Discord.Net.Core/Utils/TokenUtils.cs +++ b/src/Discord.Net.Core/Utils/TokenUtils.cs @@ -17,6 +17,47 @@ namespace Discord /// internal const int MinBotTokenLength = 58; + internal const char Base64Padding = '='; + + /// + /// Pads a base64-encoded string with 0, 1, or 2 '=' characters, + /// if the string is not a valid multiple of 4. + /// Does not ensure that the provided string contains only valid base64 characters. + /// Strings that already contain padding will not have any more padding applied. + /// + /// + /// A string that would require 3 padding characters is considered to be already corrupt. + /// Some older bot tokens may require padding, as the format provided by Discord + /// does not include this padding in the token. + /// + /// The base64 encoded string to pad with characters. + /// A string containing the base64 padding. + /// + /// Thrown if would require an invalid number of padding characters. + /// + /// + /// Thrown if is null, empty, or whitespace. + /// + internal static string PadBase64String(string encodedBase64) + { + if (string.IsNullOrWhiteSpace(encodedBase64)) + throw new ArgumentNullException(paramName: encodedBase64, + message: "The supplied base64-encoded string was null or whitespace."); + + // do not pad if already contains padding characters + if (encodedBase64.IndexOf(Base64Padding) != -1) + return encodedBase64; + + // based from https://stackoverflow.com/a/1228744 + var padding = (4 - (encodedBase64.Length % 4)) % 4; + if (padding == 3) + // can never have 3 characters of padding + throw new FormatException("The provided base64 string is corrupt, as it requires an invalid amount of padding."); + else if (padding == 0) + return encodedBase64; + return encodedBase64.PadRight(encodedBase64.Length + padding, Base64Padding); + } + /// /// Decodes a base 64 encoded string into a ulong value. /// @@ -29,6 +70,8 @@ namespace Discord try { + // re-add base64 padding if missing + encoded = PadBase64String(encoded); // decode the base64 string var bytes = Convert.FromBase64String(encoded); var idStr = Encoding.UTF8.GetString(bytes); @@ -46,7 +89,7 @@ namespace Discord } catch (ArgumentException) { - // ignore exception, can be thrown by BitConverter + // ignore exception, can be thrown by BitConverter, or by PadBase64String } return null; } diff --git a/test/Discord.Net.Tests/Tests.TokenUtils.cs b/test/Discord.Net.Tests/Tests.TokenUtils.cs index 9a1102ec5..d9ed60ae8 100644 --- a/test/Discord.Net.Tests/Tests.TokenUtils.cs +++ b/test/Discord.Net.Tests/Tests.TokenUtils.cs @@ -77,6 +77,8 @@ namespace Discord // 59 char token [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs")] [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWss")] + // simulated token with a very old user id + [InlineData("ODIzNjQ4MDEzNTAxMDcxMzY=.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKW")] public void TestBotTokenDoesNotThrowExceptions(string token) { // This example token is pulled from the Discord Docs @@ -151,6 +153,10 @@ namespace Discord // cannot pass a ulong? as a param in InlineData, so have to have a separate param // indicating if a value is null [InlineData("NDI4NDc3OTQ0MDA5MTk1NTIw", false, 428477944009195520)] + // user id that has base 64 '=' padding + [InlineData("ODIzNjQ4MDEzNTAxMDcxMzY=", false, 82364801350107136)] + // user id that does not have '=' padding, and needs it + [InlineData("ODIzNjQ4MDEzNTAxMDcxMzY", false, 82364801350107136)] // should return null w/o throwing other exceptions [InlineData("", true, 0)] [InlineData(" ", true, 0)] @@ -164,5 +170,37 @@ namespace Discord else Assert.Equal(expectedUserId, result); } + + [Theory] + [InlineData("QQ", "QQ==")] // "A" encoded + [InlineData("QUE", "QUE=")] // "AA" + [InlineData("QUFB", "QUFB")] // "AAA" + [InlineData("QUFBQQ", "QUFBQQ==")] // "AAAA" + [InlineData("QUFBQUFB", "QUFBQUFB")] // "AAAAAA" + // strings that already contain padding will be returned, even if invalid + [InlineData("QUFBQQ==", "QUFBQQ==")] + [InlineData("QUFBQQ=", "QUFBQQ=")] + [InlineData("=", "=")] + public void TestPadBase64String(string input, string expected) + { + Assert.Equal(expected, TokenUtils.PadBase64String(input)); + } + + [Theory] + // no null, empty, or whitespace + [InlineData("", typeof(ArgumentNullException))] + [InlineData(" ", typeof(ArgumentNullException))] + [InlineData("\t", typeof(ArgumentNullException))] + [InlineData(null, typeof(ArgumentNullException))] + // cannot require 3 padding chars + [InlineData("A", typeof(FormatException))] + [InlineData("QUFBQ", typeof(FormatException))] + public void TestPadBase64StringException(string input, Type type) + { + Assert.Throws(type, () => + { + TokenUtils.PadBase64String(input); + }); + } } }