Skip to content

Commit a0847e7

Browse files
github-actions[bot]buyaa-nstephentoub
authored
[release/9.0] Fix bug in validating unused bits (#107321)
* Fix bug in validating unused bits * Fix another failure * Update src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Url/Base64UrlValidator.cs Co-authored-by: Stephen Toub <[email protected]> --------- Co-authored-by: Buyaa Namnan <[email protected]> Co-authored-by: Buyaa Namnan <[email protected]> Co-authored-by: Stephen Toub <[email protected]>
1 parent d02256f commit a0847e7

File tree

3 files changed

+58
-23
lines changed

3 files changed

+58
-23
lines changed

src/libraries/System.Memory/tests/Base64/Base64ValidationUnitTests.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,29 @@ namespace System.Buffers.Text.Tests
99
{
1010
public class Base64ValidationUnitTests : Base64TestBase
1111
{
12+
[Theory]
13+
[InlineData("= ")]
14+
[InlineData("= =")]
15+
[InlineData("+ +=")]
16+
[InlineData("A=")]
17+
[InlineData("A==")]
18+
[InlineData("44==")]
19+
[InlineData(" A==")]
20+
[InlineData("AAAAA ==")]
21+
[InlineData("\tLLLL\t=\r")]
22+
[InlineData("6066=")]
23+
[InlineData("6066==")]
24+
[InlineData("SM==")]
25+
[InlineData("SM =")]
26+
[InlineData("s\rEs\r\r==")]
27+
public void BasicValidationEdgeCaseScenario(string base64UrlText)
28+
{
29+
Assert.False(Base64.IsValid(base64UrlText.AsSpan(), out int decodedLength));
30+
Assert.Equal(0, decodedLength);
31+
Span<byte> dest = new byte[Base64.GetMaxDecodedFromUtf8Length(base64UrlText.Length)];
32+
Assert.Equal(OperationStatus.InvalidData, Base64.DecodeFromUtf8(base64UrlText.ToUtf8Span(), dest, out _, out _));
33+
}
34+
1235
[Fact]
1336
public void BasicValidationBytes()
1437
{

src/libraries/System.Memory/tests/Base64Url/Base64UrlValidationUnitTests.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace System.Buffers.Text.Tests
1010
public class Base64UrlValidationUnitTests : Base64TestBase
1111
{
1212
[Theory]
13+
[InlineData("=")]
1314
[InlineData("==")]
1415
[InlineData("-%")]
1516
[InlineData("A=")]
@@ -19,10 +20,17 @@ public class Base64UrlValidationUnitTests : Base64TestBase
1920
[InlineData("AAAAA ==")]
2021
[InlineData("\tLLLL\t=\r")]
2122
[InlineData("6066=")]
23+
[InlineData("6066==")]
24+
[InlineData("SM==")]
25+
[InlineData("SM=")]
26+
[InlineData("sEs==")]
27+
[InlineData("s\rEs\r\r==")]
2228
public void BasicValidationEdgeCaseScenario(string base64UrlText)
2329
{
2430
Assert.False(Base64Url.IsValid(base64UrlText.AsSpan(), out int decodedLength));
2531
Assert.Equal(0, decodedLength);
32+
Span<byte> dest = new byte[Base64Url.GetMaxDecodedLength(base64UrlText.Length)];
33+
Assert.Equal(OperationStatus.InvalidData, Base64Url.DecodeFromChars(base64UrlText.AsSpan(), dest, out _, out _));
2634
}
2735

2836
[Fact]
@@ -258,6 +266,10 @@ public void ValidateWithPaddingReturnsCorrectCountChars(string utf8WithByteToBeI
258266
Assert.True(Base64Url.IsValid(utf8BytesWithByteToBeIgnored));
259267
Assert.True(Base64Url.IsValid(utf8BytesWithByteToBeIgnored, out int decodedLength));
260268
Assert.Equal(expectedLength, decodedLength);
269+
270+
Span<byte> dest = new byte[Base64Url.GetMaxDecodedLength(utf8WithByteToBeIgnored.Length)];
271+
Assert.Equal(OperationStatus.Done, Base64Url.DecodeFromChars(utf8WithByteToBeIgnored.AsSpan(), dest, out _, out decodedLength));
272+
Assert.Equal(expectedLength, decodedLength);
261273
}
262274

263275
[Theory]

src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Url/Base64UrlValidator.cs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Diagnostics;
45
using System.Runtime.CompilerServices;
5-
using static System.Buffers.Text.Base64Helper;
66

77
namespace System.Buffers.Text
88
{
@@ -94,33 +94,33 @@ public bool ValidateAndDecodeLength(char lastChar, int length, int paddingCount,
9494
[MethodImpl(MethodImplOptions.AggressiveInlining)]
9595
public bool ValidateAndDecodeLength(byte lastChar, int length, int paddingCount, out int decodedLength)
9696
{
97-
// Padding is optional for Base64Url, so need to account remainder. If remainder is 1, then it's invalid.
98-
#if NET
99-
(uint whole, uint remainder) = uint.DivRem((uint)(length), 4);
100-
if (remainder == 1 || (remainder > 1 && (remainder - paddingCount == 1 || paddingCount == remainder)))
101-
{
102-
decodedLength = 0;
103-
return false;
104-
}
105-
106-
decodedLength = (int)((whole * 3) + (remainder > 0 ? remainder - 1 : 0) - paddingCount);
107-
#else
97+
// Padding is optional for Base64Url, so need to account remainder.
10898
int remainder = (int)((uint)length % 4);
109-
if (remainder == 1 || (remainder > 1 && (remainder - paddingCount == 1 || paddingCount == remainder)))
99+
100+
if (paddingCount != 0)
110101
{
111-
decodedLength = 0;
112-
return false;
102+
length -= paddingCount;
103+
remainder = (int)((uint)length % 4);
104+
105+
// if there is a padding, there should be remainder and the sum of remainder and padding should not exceed 4
106+
if (remainder == 0 || remainder + paddingCount > 4)
107+
{
108+
decodedLength = 0;
109+
return false;
110+
}
113111
}
114112

115-
decodedLength = (length >> 2) * 3 + (remainder > 0 ? remainder - 1 : 0) - paddingCount;
116-
#endif
117-
int decoded = default(Base64DecoderByte).DecodingMap[lastChar];
118-
if (((remainder == 3 || paddingCount == 1) && (decoded & 0x03) != 0) ||
119-
((remainder == 2 || paddingCount == 2) && (decoded & 0x0F) != 0))
113+
decodedLength = (length >> 2) * 3 + (remainder > 0 ? remainder - 1 : 0);
114+
115+
if (remainder > 0)
120116
{
121-
// unused lower bits are not 0, reject input
122-
decodedLength = 0;
123-
return false;
117+
int decoded = default(Base64UrlDecoderByte).DecodingMap[lastChar];
118+
switch (remainder)
119+
{
120+
case 1: return false; // 1 byte is not decodable => invalid.
121+
case 2: return ((decoded & 0x0F) == 0); // if unused lower 4 bits are set to 0
122+
case 3: return ((decoded & 0x03) == 0); // if unused lower 2 bits are set to 0
123+
}
124124
}
125125

126126
return true;

0 commit comments

Comments
 (0)