Skip to content

Commit

Permalink
feature: Check for whitespace or newline characters in tokens (#1305)
Browse files Browse the repository at this point in the history
* Trim whitespace from tokens before logging in

This change trims whitespace characters from the supplied token before it is used to log in. Users can encounter this accidentally if they read their token from a file that ends with a blank line.

Leading whitespace will make the token invalid. Trailing whitespace or \n (not \r\n) will also fail to log in. \r\n (CRLF) doesn't fail because of the line break style for http request headers.

* revert trimming api token

* add check for whitespace or newline characters to existing token validation

Checks to see if a token contains any illegal characters, like whitespace or a newline. If it is, throws an ArgumentException warning the user that their token may be invalid.

I considered only checking the first and last character, but given that a token containing whitespace or a newline wouldn't work either I figured this made sense.

* removed unused usings

These were leftover from a previous approach using an ImmutableHashSet
  • Loading branch information
Chris-Johnston authored and foxbot committed May 13, 2019
1 parent 0275f7d commit bb61efa
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
22 changes: 22 additions & 0 deletions src/Discord.Net.Core/Utils/TokenUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,25 @@ internal static bool CheckBotTokenValidity(string message)
return DecodeBase64UserId(segments[0]).HasValue;
}

/// <summary>
/// The set of all characters that are not allowed inside of a token.
/// </summary>
internal static char[] IllegalTokenCharacters = new char[]
{
' ', '\t', '\r', '\n'
};

/// <summary>
/// Checks if the given token contains a whitespace or newline character
/// that would fail to log in.
/// </summary>
/// <param name="token"> The token to validate. </param>
/// <returns>
/// True if the token contains a whitespace or newline character.
/// </returns>
internal static bool CheckContainsIllegalCharacters(string token)
=> token.IndexOfAny(IllegalTokenCharacters) != -1;

/// <summary>
/// Checks the validity of the supplied token of a specific type.
/// </summary>
Expand All @@ -131,6 +150,9 @@ public static void ValidateToken(TokenType tokenType, string token)
// A Null or WhiteSpace token of any type is invalid.
if (string.IsNullOrWhiteSpace(token))
throw new ArgumentNullException(paramName: nameof(token), message: "A token cannot be null, empty, or contain only whitespace.");
// ensure that there are no whitespace or newline characters
if (CheckContainsIllegalCharacters(token))
throw new ArgumentException(message: "The token contains a whitespace or newline character. Ensure that the token has been properly trimmed.", paramName: nameof(token));

switch (tokenType)
{
Expand Down
10 changes: 10 additions & 0 deletions test/Discord.Net.Tests/Tests.TokenUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ public void TestBotTokenDoesNotThrowExceptions(string token)
[InlineData("937it3ow87i4ery69876wqire")]
// 57 char bot token
[InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kK")]
// ends with invalid characters
[InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k ")]
[InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k\n")]
[InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k\t")]
[InlineData("MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k\r\n")]
// starts with invalid characters
[InlineData(" MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k")]
[InlineData("\nMTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k")]
[InlineData("\tMTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k")]
[InlineData("\r\nMTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7k")]
[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")]
Expand Down

0 comments on commit bb61efa

Please sign in to comment.