Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IndexOutOfRangeException on decoding base64 bytes with LF characters included (>= .NET 5 only) #76918

Closed
paatrofimov opened this issue Oct 12, 2022 · 8 comments · Fixed by #82148

Comments

@paatrofimov
Copy link

paatrofimov commented Oct 12, 2022

Description

I have upgraded runtime from netcoreapp3.1 to NET 6 and the code which was transforming base64 bytes containing LF characters to string without errors started to crash with IndexOutOfRangeException. Behavior is the same for Windows and Linux platforms.

Reproduction Steps

[TestFixture]
public class Fixture
{
    [Test]
    public void Crashes_on_runtime_greater_or_equal_to_NET_5()
    {
        var txt = "YWJj\nZGVm"; // abc\ndef
        var base64Bytes = Encoding.UTF8.GetBytes(txt);
        var stream = new MemoryStream(base64Bytes);
        var base64Transform = new FromBase64Transform();
        var cryptoStream = new CryptoStream(stream, base64Transform, CryptoStreamMode.Read);

        var result = new MemoryStream();
        cryptoStream.CopyTo(result);

        Console.WriteLine(Encoding.UTF8.GetString(result.ToArray()));
    }
}

Expected behavior

"abcdef" in the output, which is happening for example on netcoreapp3.1 runtime.

Actual behavior

System.IndexOutOfRangeException : Index was outside the bounds of the array.
   at System.Security.Cryptography.FromBase64Transform.TransformFinalBlock(Byte[] inputBuffer, Int32 inputOffset, Int32 inputCount)
   at System.Security.Cryptography.CryptoStream.ReadAsyncCore(Memory`1 buffer, CancellationToken cancellationToken, Boolean useAsync)
   at System.Security.Cryptography.CryptoStream.Read(Byte[] buffer, Int32 offset, Int32 count)
   at System.Security.Cryptography.CryptoStream.CopyTo(Stream destination, Int32 bufferSize)
   at System.IO.Stream.CopyTo(Stream destination)
   at ClassLibrary1.Fixture.Crashes_on_runtime_greater_or_equal_to_NET_5() in E:\cm_1\drive\ClassLibrary1\Class1.cs:line 20

Regression?

Regression happened after the upgrade from netcoreapp3.1 to NET 5/NET 6.

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 12, 2022
@ghost
Copy link

ghost commented Oct 12, 2022

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

I have upgraded runtime from netcoreapp3.1 to NET 6 and the code which was transforming base64 bytes containing LF characters to string without errors started to crash with IndexOutOfRangeException. Behavior is the same for Windows and Linux platforms.

Reproduction Steps

[TestFixture]
public class Fixture
{
    [Test]
    public void Crashes_on_runtime_greater_or_equal_to_NET_5()
    {
        var txt = "YWJj\nZGVm"; // abc\ndef
        var base64Bytes = Encoding.UTF8.GetBytes(txt);
        var stream = new MemoryStream(base64Bytes);
        var base64Transform = new FromBase64Transform();
        var cryptoStream = new CryptoStream(stream, base64Transform, CryptoStreamMode.Read);

        var result = new MemoryStream();
        cryptoStream.CopyTo(result);

        Console.WriteLine(Encoding.UTF8.GetString(result.ToArray()));
    }
}

Expected behavior

"abcdef" in the output, which is happening for example on netcoreapp3.1 runtime.

Actual behavior

System.IndexOutOfRangeException : Index was outside the bounds of the array.
   at System.Security.Cryptography.FromBase64Transform.TransformFinalBlock(Byte[] inputBuffer, Int32 inputOffset, Int32 inputCount)
   at System.Security.Cryptography.CryptoStream.ReadAsyncCore(Memory`1 buffer, CancellationToken cancellationToken, Boolean useAsync)
   at System.Security.Cryptography.CryptoStream.Read(Byte[] buffer, Int32 offset, Int32 count)
   at System.Security.Cryptography.CryptoStream.CopyTo(Stream destination, Int32 bufferSize)
   at System.IO.Stream.CopyTo(Stream destination)
   at ClassLibrary1.Fixture.Crashes_on_runtime_greater_or_equal_to_NET_5() in E:\cm_1\drive\ClassLibrary1\Class1.cs:line 20

Regression?

Regression happened after the upgrade from netcoreapp3.1 to NET 6.

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: paatrofimov
Assignees: -
Labels:

area-System.Security

Milestone: -

@paatrofimov
Copy link
Author

It is worth mentioning that the same input but using a CRLF delimiter works without exceptions.
The problem only occurs with the LF delimiter.

@gfoidl
Copy link
Member

gfoidl commented Oct 12, 2022

The crash happens in

when not enough bytes are in tmpBuffer which is to happen with \n in your example (len=1), whilst for \r\n len=2 (or any other whitespace instead of \r).
So the problem is in FromBase64Transform (test-hole?).

Side note: Base64.DecodeFromUtf8 returns InvalidData (consumed=4, written=3) as expected and documented.
And for completeness cf. #76020 (comment)

@gfoidl
Copy link
Member

gfoidl commented Oct 12, 2022

Having another look with the input YWJj\nZGVm (in bytes: 0x59, 0x57, 0x4a, 0x6a, 0x0a, 0x5a, 0x47, 0x56, 0x6d):

  1. TransformBlock with inputCount = 8 is called -- so the \n is part of that block
  2. after removing whitespace 7 elements remain in that block (0x59, 0x57, 0x4a, 0x6a, 0x5a, 0x47, 0x56)
  3. decoded is up to last multiple of 4 (restriction of base64), so first 4 bytes decoded, last 3 bytes saved to _inputBuffer --> succeeds
  4. TransformFinalBlock is called with inputBuffer existing of 0x6d, 0, 0, 0 and inputCount = 1

Point 4 is the problem here.
Correct would be combining the _inputBuffer (remainder from previous block decode) with the new inputBuffer, so that there are the remaining 4 bytes which can be decoded correctly.

For futher analysis I don't have time now...

@paatrofimov
Copy link
Author

FYI guys from SO suggested padding base64 bytes to the length of % 4 as a workaround until this is fixed. It seems to work and the implementation is simpler than adding a middleware stream which would remove all whitespaces from base64 bytes input.

@CodeCasterNL
Copy link

@gfoidl we arrived to the same conclusion, see https://stackoverflow.com/questions/74039771/indexoutofrangeexception-on-decoding-base64-bytes-with-lf-characters-included/74041328#74041328.

@gfoidl
Copy link
Member

gfoidl commented Oct 12, 2022

OK. But why was here no link to to SO given before as that had saved a bit of work? And vice-versa.

@paatrofimov
Copy link
Author

Yeah, sorry about the links, just thought I'd launch SO and GitHub issue processes in parallel, so the solution would be found faster.
Is it possible to find out when will this be fixed, approximately, at least?

@jozkee jozkee added this to the 8.0.0 milestone Feb 13, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 13, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 15, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 23, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants