From 13ec96e657eddc72a71ffd3d7240a74d112a0c59 Mon Sep 17 00:00:00 2001 From: Yann Crumeyrolle Date: Mon, 16 Aug 2021 21:28:30 +0200 Subject: [PATCH] Fix Base64 decoding when whitespaces occur (#571) * Fix Base64 decoding when whitespaces occur * Fix JWK validation of the field 'x5c'. This field is b64-encoded and not b64-url encoded. --- src/JsonWebToken/Base64.cs | 92 +++++++++++++--------- src/JsonWebToken/Base64Url.cs | 51 ++++++------ src/JsonWebToken/Jwk.cs | 5 +- test/JsonWebToken.Tests.Common/Tokens.cs | 28 +++---- test/JsonWebToken.Tests/Base64Tests.cs | 44 +++++++++++ test/JsonWebToken.Tests/Base64UrlTests.cs | 38 +++++++++ test/JsonWebToken.Tests/JsonWebKeyTests.cs | 8 +- 7 files changed, 187 insertions(+), 79 deletions(-) create mode 100644 test/JsonWebToken.Tests/Base64Tests.cs create mode 100644 test/JsonWebToken.Tests/Base64UrlTests.cs diff --git a/src/JsonWebToken/Base64.cs b/src/JsonWebToken/Base64.cs index a76b03721..7a35f4237 100644 --- a/src/JsonWebToken/Base64.cs +++ b/src/JsonWebToken/Base64.cs @@ -79,7 +79,11 @@ public static int Decode(ReadOnlySpan base64, Span data) public static OperationStatus Decode(ReadOnlySpan base64, Span data, out int bytesConsumed, out int bytesWritten) { int lastWhitespace = base64.LastIndexOfAny(WhiteSpace); - if (lastWhitespace != -1) + if (lastWhitespace == -1) + { + return gfoidl.Base64.Base64.Default.Decode(base64, data, out bytesConsumed, out bytesWritten); + } + else { byte[]? utf8ArrayToReturn = null; Span utf8Data = base64.Length > Constants.MaxStackallocBytes @@ -87,20 +91,34 @@ public static OperationStatus Decode(ReadOnlySpan base64, Span data, : stackalloc byte[Constants.MaxStackallocBytes]; try { + int firstWhitespace = base64.IndexOfAny(WhiteSpace); int length = 0; - int i = 0; - for (; i <= lastWhitespace; i++) + Span buffer = utf8Data; + if (firstWhitespace != lastWhitespace) { - var current = base64[i]; - if (!IsWhiteSpace(current)) + while (firstWhitespace != -1) { - utf8Data[length++] = current; + base64.Slice(0, firstWhitespace).CopyTo(buffer); + buffer = buffer.Slice(firstWhitespace); + length += firstWhitespace; + + // Skip whitespaces + int i = firstWhitespace; + while (++i < base64.Length && IsWhiteSpace(base64[i])) ; + + base64 = base64.Slice(i); + firstWhitespace = base64.IndexOfAny(WhiteSpace); } - } - for (; i < base64.Length; i++) + //// Copy the remaining + base64.CopyTo(buffer); + length += base64.Length; + } + else { - utf8Data[length++] = base64[i]; + base64.Slice(0, firstWhitespace).CopyTo(buffer); + base64.Slice(firstWhitespace + 1).CopyTo(buffer.Slice(firstWhitespace)); + length = base64.Length - 1; } return gfoidl.Base64.Base64.Default.Decode(utf8Data.Slice(0, length), data, out bytesConsumed, out bytesWritten); @@ -113,16 +131,13 @@ public static OperationStatus Decode(ReadOnlySpan base64, Span data, } } } - else - { - return gfoidl.Base64.Base64.Default.Decode(base64, data, out bytesConsumed, out bytesWritten); - } } private static bool IsWhiteSpace(byte c) => c == ' ' || (c >= '\t' && c <= '\r'); - private static ReadOnlySpan WhiteSpace => new byte[] { (byte)' ', (byte)'\t', (byte)'\r', (byte)'\n', (byte)'\v', (byte)'\f' }; + private static ReadOnlySpan WhiteSpace + => new byte[] { (byte)' ', (byte)'\t', (byte)'\n', (byte)'\v', (byte)'\f', (byte)'\r' }; /// Encodes a span of UTF-8 text into a span of bytes. /// The number of the bytes written to . @@ -225,33 +240,36 @@ internal static unsafe bool IsBase64String(ReadOnlySpan value) static bool IsValidBase64Char(char value) { - if (value > byte.MaxValue) + bool result = false; + if (value <= byte.MaxValue) { - return false; - } - - byte byteValue = (byte)value; + byte byteValue = (byte)value; - // 0-9 - if (byteValue >= (byte)'0' && byteValue <= (byte)'9') - { - return true; - } - - // + or / - if (byteValue == (byte)'+' || byteValue == (byte)'/') - { - return true; - } - - // a-z or A-Z - byteValue |= 0x20; - if (byteValue >= (byte)'a' && byteValue <= (byte)'z') - { - return true; + // 0-9 + if (byteValue >= (byte)'0' && byteValue <= (byte)'9') + { + result = true; + } + else + { + // a-z or A-Z + byte letter = (byte)(byteValue | 0x20); + if (letter >= (byte)'a' && letter <= (byte)'z') + { + result = true; + } + else + { + // + or / or whitespaces + if (byteValue == (byte)'+' || byteValue == (byte)'/' || IsWhiteSpace(byteValue)) + { + result = true; + } + } + } } - return false; + return result; } } } diff --git a/src/JsonWebToken/Base64Url.cs b/src/JsonWebToken/Base64Url.cs index 5335be1b9..1d1937200 100644 --- a/src/JsonWebToken/Base64Url.cs +++ b/src/JsonWebToken/Base64Url.cs @@ -210,33 +210,36 @@ internal static unsafe bool IsBase64UrlString(ReadOnlySpan value) static bool IsValidBase64UrlChar(char value) { - if (value > byte.MaxValue) + bool result = false; + if (value <= byte.MaxValue) { - return false; - } - - byte byteValue = (byte)value; - - // 0-9 - if (byteValue >= (byte)'0' && byteValue <= (byte)'9') - { - return true; - } - - // - or _ - if (byteValue == (byte)'-' || byteValue == (byte)'_') - { - return true; - } - - // a-z or A-Z - byteValue |= 0x20; - if (byteValue >= (byte)'a' && byteValue <= (byte)'z') - { - return true; + byte byteValue = (byte)value; + + // 0-9 + if (byteValue >= (byte)'0' && byteValue <= (byte)'9') + { + result = true; + } + else + { + // a-z or A-Z + byte letter = (byte)(byteValue | 0x20); + if (letter >= (byte)'a' && letter <= (byte)'z') + { + result = true; + } + else + { + // - or _ + if (byteValue == (byte)'-' || byteValue == (byte)'_') + { + result = true; + } + } + } } - return false; + return result; } } } diff --git a/src/JsonWebToken/Jwk.cs b/src/JsonWebToken/Jwk.cs index f28f212e4..260bc4b0f 100644 --- a/src/JsonWebToken/Jwk.cs +++ b/src/JsonWebToken/Jwk.cs @@ -1096,7 +1096,7 @@ public static void Validate(string json) throw new JwkValidationException($"Invalid '{JwkParameterNames.X5c}' item. Must be of type 'String'. Value '{item.GetRawText()}' is of type '{item.ValueKind}'."); } - if (Base64Url.IsBase64UrlString(item.GetString()!)) + if (!Base64.IsBase64String(item.GetString()!)) { throw new JwkValidationException($"Invalid '{JwkParameterNames.X5c}' value '{item.GetString()}'. Must be a valid base64 encoded string."); } @@ -1106,7 +1106,6 @@ public static void Validate(string json) CheckOptionalBase64UrlMember(document, JwkParameterNames.X5t, 160); CheckOptionalBase64UrlMember(document, JwkParameterNames.X5tS256, 256); CheckOptionalStringMember(document, JwkParameterNames.X5u); - } catch (JsonException e) { @@ -1125,7 +1124,7 @@ static void CheckRequiredBase64UrlMember(JsonDocument document, JsonEncodedText throw new JwkValidationException($"Invalid '{memberName}' member. Must be of type 'String'. Value '{value.GetRawText()}' is of type '{value.ValueKind}'."); } - if (!Base64.IsBase64String(value.GetString()!)) + if (!Base64Url.IsBase64UrlString(value.GetString()!)) { throw new JwkValidationException($"Invalid '{memberName}' member. Must be a base64-URL encoded string."); } diff --git a/test/JsonWebToken.Tests.Common/Tokens.cs b/test/JsonWebToken.Tests.Common/Tokens.cs index 383132699..2720346db 100644 --- a/test/JsonWebToken.Tests.Common/Tokens.cs +++ b/test/JsonWebToken.Tests.Common/Tokens.cs @@ -60,10 +60,10 @@ private static IDictionary> CreatePayloads() { { "jti", "756E69717565206964656E746966696572"}, { "iss", "https://idp.example.com/"}, - { "iat", 1508184845}, + { "iat", EpochTime.UtcNow }, { "aud", "636C69656E745F6964"}, - { "exp", 1628184845}, - { "nbf", 1508184845} + { "exp", EpochTime.UtcNow + EpochTime.OneDay }, + { "nbf", EpochTime.UtcNow } } }, { @@ -71,10 +71,10 @@ private static IDictionary> CreatePayloads() { { "jti", "756E69717565206964656E746966696572"}, { "iss", "https://idp.example.com/"}, - { "iat", 1508184845}, + { "iat", EpochTime.UtcNow }, { "aud", new JArray("636C69656E745F6964", "X", "Y" ) }, - { "exp", 1628184845}, - { "nbf", 1508184845} + { "exp", EpochTime.UtcNow + EpochTime.OneDay}, + { "nbf", EpochTime.UtcNow } } }, { @@ -82,9 +82,9 @@ private static IDictionary> CreatePayloads() { { "jti", "756E69717565206964656E746966696572"}, { "iss", "https://idp.example.com/"}, - { "iat", 1508184845}, + { "iat", EpochTime.UtcNow }, { "aud", "636C69656E745F6964"}, - { "exp", 1628184845} + { "exp", EpochTime.UtcNow + EpochTime.OneDay} } }, { @@ -92,10 +92,10 @@ private static IDictionary> CreatePayloads() { { "jti", "756E69717565206964656E746966696572"}, { "iss", "https://idp.example.com/"}, - { "iat", 1508184845}, + { "iat", EpochTime.UtcNow }, { "aud", "636C69656E745F6964"}, - { "exp", 1628184845}, - { "nbf", 1508184845}, + { "exp", EpochTime.UtcNow + EpochTime.OneDay}, + { "nbf", EpochTime.UtcNow }, { "claim1", "value1ABCDEFGH" }, { "claim2", "value1ABCDEFGH" }, { "claim3", "value1ABCDEFGH" }, @@ -119,10 +119,10 @@ private static IDictionary> CreatePayloads() { { "jti", "756E69717565206964656E746966696572" }, { "iss", "https://idp.example.com/" }, - { "iat", 1508184845 }, + { "iat", EpochTime.UtcNow }, { "aud", "636C69656E745F6964" }, - { "exp", 1628184845 }, - { "nbf", 1508184845}, + { "exp", EpochTime.UtcNow + EpochTime.OneDay }, + { "nbf", EpochTime.UtcNow }, { "big_claim", Convert.ToBase64String(bigData) } } }, diff --git a/test/JsonWebToken.Tests/Base64Tests.cs b/test/JsonWebToken.Tests/Base64Tests.cs new file mode 100644 index 000000000..012c96f32 --- /dev/null +++ b/test/JsonWebToken.Tests/Base64Tests.cs @@ -0,0 +1,44 @@ +using System; +using System.Text; +using Xunit; + +namespace JsonWebToken.Tests +{ + public class Base64Tests + { + [Theory] + [InlineData("", "")] + [InlineData("SGVsbG8=", "Hello")] + [InlineData("SGVsbG8gV29ybGQ=", "Hello World")] + [InlineData("SGVsbG8\tgV29ybGQ=", "Hello World")] + [InlineData("SGVsbG8\rgV29ybGQ=", "Hello World")] + [InlineData("SGVsbG8\ngV29ybGQ=", "Hello World")] + [InlineData("SGVsbG8\vgV29ybGQ=", "Hello World")] + [InlineData("SGVsbG8\fgV29ybGQ=", "Hello World")] + [InlineData("SGVsbG8 gV29ybGQ=", "Hello World")] + [InlineData(" SGVsbG8gV29ybGQ=", "Hello World")] + [InlineData("SG Vsb G8gV29ybGQ=", "Hello World")] + [InlineData("S G V s b G 8 g V 2 9 y b G Q =", "Hello World")] + [InlineData("S G V s b G8gV29ybGQ=", "Hello World")] + [InlineData(" S G V s b G8gV29ybGQ= ", "Hello World")] + [InlineData("SGV+bG8=", "He~lo")] + [InlineData("SGV/bG8=", "He\u007flo")] + public void Decode_Valid(string value, string expected) + { + var result = Base64.Decode(Encoding.UTF8.GetBytes(value)); + Assert.NotNull(result); + Assert.Equal(Encoding.UTF8.GetBytes(expected), result); + } + + [Theory] + [InlineData("SGVsbG8")] + [InlineData("SGVsbG8&")] + [InlineData("SGVsbG=8")] + [InlineData("S-/sbG8=")] + [InlineData("S+_sbG8=")] + public void Decode_Invalid(string value) + { + Assert.Throws(() => Base64.Decode(Encoding.UTF8.GetBytes(value))); + } + } +} diff --git a/test/JsonWebToken.Tests/Base64UrlTests.cs b/test/JsonWebToken.Tests/Base64UrlTests.cs new file mode 100644 index 000000000..cc06c3135 --- /dev/null +++ b/test/JsonWebToken.Tests/Base64UrlTests.cs @@ -0,0 +1,38 @@ +using System; +using System.Text; +using Xunit; + +namespace JsonWebToken.Tests +{ + public class Base64UrlTests + { + [Theory] + [InlineData("", "")] + [InlineData("SGVsbG8", "Hello")] + [InlineData("SGVsbG8gV29ybGQ", "Hello World")] + [InlineData("SGV-bG8", "He~lo")] + [InlineData("SGV_bG8", "He\u007flo")] + public void Decode_Valid(string value, string expected) + { + var result = Base64Url.Decode(Encoding.UTF8.GetBytes(value)); + Assert.NotNull(result); + Assert.Equal(Encoding.UTF8.GetBytes(expected), result); + } + + [Theory] + [InlineData("SGVsbG8=")] + [InlineData("SGVsbG8 ")] + [InlineData(" SGVsbG8")] + [InlineData("SGV sbG8")] + public void Decode_Invalid(string value) + { + Assert.Throws(() => Base64Url.Decode(Encoding.UTF8.GetBytes(value))); + } + + [Fact] + public void Decode_Null() + { + Assert.Throws(() => Base64Url.Decode((string)null)); + } + } +} diff --git a/test/JsonWebToken.Tests/JsonWebKeyTests.cs b/test/JsonWebToken.Tests/JsonWebKeyTests.cs index 9ba16ea7b..de62f6fa1 100644 --- a/test/JsonWebToken.Tests/JsonWebKeyTests.cs +++ b/test/JsonWebToken.Tests/JsonWebKeyTests.cs @@ -189,6 +189,9 @@ public void Thumbprint() [Theory] public void Validate_Valid_DoesNotThrowException(string description, string json) { + Jwk.Validate(json); + Assert.True(true, description); + var jwk = Jwk.FromJson(json); jwk.Validate(); Assert.True(true, description); @@ -500,6 +503,9 @@ public void Validate_Valid_DoesNotThrowException(string description, string json [Theory] public void Validate_Invalid_ThrowsJwkCheckException(string description, string json) { + Assert.ThrowsAny(() => Jwk.Validate(json)); + Assert.True(true, description); + var jwk = Jwk.FromJson(json); Assert.ThrowsAny(() => jwk.Validate()); Assert.True(true, description); @@ -1082,7 +1088,7 @@ public void Check_Valid(string description, string json) { ""kty"": ""oct"", ""k"": ""Sm7nSOWIqLc8xMK5CRhEiePi9iNukStXhssrYdSiMk0"", - ""x5c"": [""fake""], + ""x5c"": [""####""], ""x5t"": ""qUqP5cyxm6YcTAhz05Hph5gvu9M"", ""x5t#S256"": ""n4bQgYhMfWWaL-qgxVrQFaO_TxsrC4Is0V1sFbDwCgg"", ""x5u"": ""https://example.com""