fix: Improve validation of Bot Tokens (#1206)

* improve bot token validation by trying to decode user id from token

Try to decode the user id from the supplied bot token as a way of validating the token. If this should fail, indicate that the token is invalid.

* Update the tokenutils tests to pass the new validation checks

* Add test case for CheckBotTokenValidity method

* lint: clean up whitespace

* Add check for null or whitespace string, lint whitespace

* fix userid conversion

* Add hint to user to check that token is not an oauth client secret

* Catch exception that can be thrown by GetString

* Refactor token conversion logic into it's own testable method

src/Discord.Net.Core/Utils/TokenUtils.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Text;
 
 namespace Discord
 {
@@ -17,6 +18,65 @@ namespace Discord
         internal const int MinBotTokenLength = 58;
 
         /// <summary>
+        ///     Decodes a base 64 encoded string into a ulong value.
+        /// </summary>
+        /// <param name="encoded"> A base 64 encoded string containing a User Id.</param>
+        /// <returns> A ulong containing the decoded value of the string, or null if the value was invalid. </returns>
+        internal static ulong? DecodeBase64UserId(string encoded)
+        {
+            if (string.IsNullOrWhiteSpace(encoded))
+                return null;
+
+            try
+            {
+                // decode the base64 string
+                var bytes = Convert.FromBase64String(encoded);
+                var idStr = Encoding.UTF8.GetString(bytes);
+                // try to parse a ulong from the resulting string
+                if (ulong.TryParse(idStr, out var id))
+                    return id;
+            }
+            catch (DecoderFallbackException)
+            {
+                // ignore exception, can be thrown by GetString
+            }
+            catch (FormatException)
+            {
+                // ignore exception, can be thrown if base64 string is invalid
+            }
+            catch (ArgumentException)
+            {
+                // ignore exception, can be thrown by BitConverter
+            }
+            return null;
+        }
+
+        /// <summary>
+        ///     Checks the validity of a bot token by attempting to decode a ulong userid
+        ///     from the bot token.
+        /// </summary>
+        /// <param name="message">
+        ///     The bot token to validate.
+        /// </param>
+        /// <returns>
+        ///     True if the bot token was valid, false if it was not.
+        /// </returns>
+        internal static bool CheckBotTokenValidity(string message)
+        {
+            if (string.IsNullOrWhiteSpace(message))
+                return false;
+
+            // split each component of the JWT
+            var segments = message.Split('.');
+
+            // ensure that there are three parts
+            if (segments.Length != 3)
+                return false;
+            // return true if the user id could be determined
+            return DecodeBase64UserId(segments[0]).HasValue;
+        }
+
+        /// <summary>
         ///     Checks the validity of the supplied token of a specific type.
         /// </summary>
         /// <param name="tokenType"> The type of token to validate. </param>
@@ -42,13 +102,17 @@ namespace Discord
                     // this value was determined by referencing examples in the discord documentation, and by comparing with
                     // pre-existing tokens
                     if (token.Length < MinBotTokenLength)
-                        throw new ArgumentException(message: $"A Bot token must be at least {MinBotTokenLength} characters in length.", paramName: nameof(token));
+                        throw new ArgumentException(message: $"A Bot token must be at least {MinBotTokenLength} characters in length. " +
+                            "Ensure that the Bot Token provided is not an OAuth client secret.", paramName: nameof(token));
+                    // check the validity of the bot token by decoding the ulong userid from the jwt
+                    if (!CheckBotTokenValidity(token))
+                        throw new ArgumentException(message: "The Bot token was invalid. " +
+                            "Ensure that the Bot Token provided is not an OAuth client secret.", paramName: nameof(token));
                     break;
                 default:
                     // All unrecognized TokenTypes (including User tokens) are considered to be invalid.
                     throw new ArgumentException(message: "Unrecognized TokenType.", paramName: nameof(token));
             }
         }
-
     }
 }

test/Discord.Net.Tests/Tests.TokenUtils.cs

@@ -76,9 +76,7 @@ namespace Discord
         [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKW")]
         // 59 char token
         [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs")]
-        [InlineData("This appears to be completely invalid, however the current validation rules are not very strict.")]
         [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWss")]
-        [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWsMTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs")]
         public void TestBotTokenDoesNotThrowExceptions(string token)
         {
             // This example token is pulled from the Discord Docs
@@ -99,6 +97,9 @@ namespace Discord
         [InlineData("937it3ow87i4ery69876wqire")]
         // 57 char bot token
         [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kK")]
+        [InlineData("This is an invalid token, but it passes the check for string length.")]
+        // valid token, but passed in twice
+        [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWsMTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs")]
         public void TestBotTokenInvalidThrowsArgumentException(string token)
         {
             Assert.Throws<ArgumentException>(() => TokenUtils.ValidateToken(TokenType.Bot, token));
@@ -124,5 +125,44 @@ namespace Discord
             Assert.Throws<ArgumentException>(() =>
                 TokenUtils.ValidateToken((TokenType)type, "MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs"));
         }
+
+        /// <summary>
+        ///     Checks the <see cref="TokenUtils.CheckBotTokenValidity(string)"/> method for expected output.
+        /// </summary>
+        /// <param name="token"> The Bot Token to test.</param>
+        /// <param name="expected"> The expected result. </param>
+        [Theory]
+        // this method only checks the first part of the JWT
+        [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4..", true)]
+        [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kK", true)]
+        [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4. this part is invalid. this part is also invalid", true)]
+        [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.", false)]
+        [InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4", false)]
+        [InlineData("NDI4NDc3OTQ0MDA5MTk1NTIw.xxxx.xxxxx", true)]
+        // should not throw an unexpected exception
+        [InlineData("", false)]
+        [InlineData(null, false)]
+        public void TestCheckBotTokenValidity(string token, bool expected)
+        {
+            Assert.Equal(expected, TokenUtils.CheckBotTokenValidity(token));
+        }
+
+        [Theory]
+        // 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)]
+        // should return null w/o throwing other exceptions
+        [InlineData("", true, 0)]
+        [InlineData(" ", true, 0)]
+        [InlineData(null, true, 0)]
+        [InlineData("these chars aren't allowed @U#)*@#!)*", true, 0)]
+        public void TestDecodeBase64UserId(string encodedUserId, bool isNull, ulong expectedUserId)
+        {
+            var result = TokenUtils.DecodeBase64UserId(encodedUserId);
+            if (isNull)
+                Assert.Null(result);
+            else
+                Assert.Equal(expectedUserId, result);
+        }
     }
 }