PEReader - Big Endian issues when getting a string

[nealef]

When using FileVersionInfo.GetVersionInfo the PEReader uses the standard code path for creating a string from an array of bytes that takes it to System.Text.ASCIIUtility:GetIndexOfFirstNonAsciiByte_Default which assumes the byte ordering is in the native order. However, PE objects such as, for example, Microsoft.Build.dll the strings are in little endian order (for example, the © is 0xa9c2 in the PE object but is 0xc2a9 when used on Big Endian systems). This leads to System.Text.ASCIIUtility:WidenAsciiToUtf16 returning a non-zero result and then eventually, to:

[ERROR] FATAL UNHANDLED EXCEPTION: System.ArgumentException: The output char buffer is too small to contain the decoded characters, encoding 'Unicode (UTF-8)' fallback 'System.Text.DecoderReplacementFallback'. (Parameter 'chars')
   at System.Text.Encoding.ThrowCharsOverflow(DecoderNLS decoder, Boolean nothingDecoded) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs:line 1179
   at System.Text.Encoding.GetCharsWithFallback(ReadOnlySpan`1 bytes, Int32 originalBytesLength, Span`1 chars, Int32 originalCharsLength, DecoderNLS decoder) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.Internal.cs:line 1272
   at System.Text.UTF8Encoding.GetCharsWithFallback(ReadOnlySpan`1 bytes, Int32 originalBytesLength, Span`1 chars, Int32 originalCharsLength, DecoderNLS decoder) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/UTF8Encoding.cs:line 647
   at System.Text.Encoding.GetCharsWithFallback(Byte* pOriginalBytes, Int32 originalByteCount, Char* pOriginalChars, Int32 originalCharCount, Int32 bytesConsumedSoFar, Int32 charsWrittenSoFar) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.Internal.cs:line 1086
   at System.Text.UTF8Encoding.GetChars(Byte* bytes, Int32 byteCount, Char* chars, Int32 charCount) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/UTF8Encoding.cs:line 553
   at System.String.CreateStringFromEncoding(Byte* bytes, Int32 byteLength, Encoding encoding) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/String.cs:line 540
   at System.Text.Encoding.GetString(Byte* bytes, Int32 byteCount) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs:line 941
   at System.Reflection.Internal.MemoryBlock.PeekUtf8(Int32 offset, Int32 byteCount) in /home/neale/runtimelab/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/MemoryBlock.cs:line 289
   at System.Reflection.Metadata.BlobReader.ReadUTF8(Int32 byteCount) in /home/neale/runtimelab/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobReader.cs:line 441
   at System.Reflection.Metadata.BlobReader.ReadSerializedString() in /home/neale/runtimelab/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobReader.cs:line 641
   at System.Diagnostics.FileVersionInfo.GetStringAttributeArgumentValue(MetadataReader reader, CustomAttribute attr, String& value) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 315
   at System.Diagnostics.FileVersionInfo.LoadManagedAssemblyMetadata(MetadataReader metadataReader, Boolean isExe) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 134
   at System.Diagnostics.FileVersionInfo.TryLoadManagedAssemblyMetadata() in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 60
   at System.Diagnostics.FileVersionInfo..ctor(String fileName) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 26
   at System.Diagnostics.FileVersionInfo.GetVersionInfo(String fileName) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.cs:line 279
   at HelloWorld.Program.Main(String[] args) in /home/neale/HelloWorld/Program.cs:line 14

The PE decoding takes place in BlobReader and most of the decoding is endian aware. This does not seem to be the case for ReadUTF8. It would seem that ReadUTF8 may need to scan the UTF8 and reorder the multibyte characters before converting to a string.

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[nealef]

I don't have the ability to add labels but would suggest: area-System.Reflection.Metadata

[jkotas]

This looks like a big-endian bug in GetIndexOfFirstNonAsciiByte_Default

EDIT: I do not see anything obviously wrong

cc @GrabYourPitchforks

[ghost]

Tagging subscribers to this area: @tarekgh, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

Description:	When using FileVersionInfo.GetVersionInfo the PEReader uses the standard code path for creating a string from an array of bytes that takes it to System.Text.ASCIIUtility:GetIndexOfFirstNonAsciiByte_Default which assumes the byte ordering is in the native order. However, PE objects such as, for example, Microsoft.Build.dll the strings are in little endian order (for example, the © is 0xa9c2 in the PE object but is 0xc2a9 when used on Big Endian systems). This leads to System.Text.ASCIIUtility:WidenAsciiToUtf16 returning a non-zero result and then eventually, to: [ERROR] FATAL UNHANDLED EXCEPTION: System.ArgumentException: The output char buffer is too small to contain the decoded characters, encoding 'Unicode (UTF-8)' fallback 'System.Text.DecoderReplacementFallback'. (Parameter 'chars') at System.Text.Encoding.ThrowCharsOverflow(DecoderNLS decoder, Boolean nothingDecoded) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs:line 1179 at System.Text.Encoding.GetCharsWithFallback(ReadOnlySpan`1 bytes, Int32 originalBytesLength, Span`1 chars, Int32 originalCharsLength, DecoderNLS decoder) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.Internal.cs:line 1272 at System.Text.UTF8Encoding.GetCharsWithFallback(ReadOnlySpan`1 bytes, Int32 originalBytesLength, Span`1 chars, Int32 originalCharsLength, DecoderNLS decoder) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/UTF8Encoding.cs:line 647 at System.Text.Encoding.GetCharsWithFallback(Byte* pOriginalBytes, Int32 originalByteCount, Char* pOriginalChars, Int32 originalCharCount, Int32 bytesConsumedSoFar, Int32 charsWrittenSoFar) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.Internal.cs:line 1086 at System.Text.UTF8Encoding.GetChars(Byte* bytes, Int32 byteCount, Char* chars, Int32 charCount) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/UTF8Encoding.cs:line 553 at System.String.CreateStringFromEncoding(Byte* bytes, Int32 byteLength, Encoding encoding) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/String.cs:line 540 at System.Text.Encoding.GetString(Byte* bytes, Int32 byteCount) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs:line 941 at System.Reflection.Internal.MemoryBlock.PeekUtf8(Int32 offset, Int32 byteCount) in /home/neale/runtimelab/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/MemoryBlock.cs:line 289 at System.Reflection.Metadata.BlobReader.ReadUTF8(Int32 byteCount) in /home/neale/runtimelab/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobReader.cs:line 441 at System.Reflection.Metadata.BlobReader.ReadSerializedString() in /home/neale/runtimelab/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobReader.cs:line 641 at System.Diagnostics.FileVersionInfo.GetStringAttributeArgumentValue(MetadataReader reader, CustomAttribute attr, String& value) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 315 at System.Diagnostics.FileVersionInfo.LoadManagedAssemblyMetadata(MetadataReader metadataReader, Boolean isExe) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 134 at System.Diagnostics.FileVersionInfo.TryLoadManagedAssemblyMetadata() in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 60 at System.Diagnostics.FileVersionInfo..ctor(String fileName) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Unix.cs:line 26 at System.Diagnostics.FileVersionInfo.GetVersionInfo(String fileName) in /home/neale/runtimelab/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.cs:line 279 at HelloWorld.Program.Main(String[] args) in /home/neale/HelloWorld/Program.cs:line 14 The PE decoding takes place in BlobReader and most of the decoding is endian aware. This does not seem to be the case for ReadUTF8. It would seem that ReadUTF8 may need to scan the UTF8 and reorder the multibyte characters before converting to a string.
Author:	nealef
Assignees:	-
Labels:	area-System.Text.Encoding, untriaged
Milestone:	-

[jkotas]

It would seem that ReadUTF8 may need to scan the UTF8 and reorder the multibyte characters

I do not see how this can be problem.

@nealef Would you be interested in proposing and testing a fix for this?

[GrabYourPitchforks]

There was a report a while back where somebody was encountering this issue, and the root cause was that the input buffer changed in the middle of the GetString operation. Could the same thing be occurring here?

Is the machine experiencing this issue a big-endian machine? Do you have a sample payload we can investigate?

[nealef]

@jkotas You are correct. The string is valid and doesn't require any endian specific fix. However, it does lead to the Exception shown above. What I have narrowed it down to are two problems:

• Garbled output when byte sequence contains characters > 0x7f
• Exception when sequence starts with 0xc9 0xa2 [not strictly only these characters but these will trigger the issue]

A Couple of Test Cases

Converting Bytes to UTF8

I created a simple test case to see what the behaviour of converting bytes to UTF8 is for little endian and big endian:

using System;
using System.Text;

namespace HelloWorld
{
    class Program
    {
        static void Main(string[] args)
        {
            Byte[] x = {0x4D, 0x69, 0x63, 0x72, 0x6F, 0x73, 0x6F, 0x66, 0x74, 0xc2, 0xae, 0x20, 0x42,
                        0x75, 0x69, 0x6C, 0x64, 0x20, 0x54, 0x6F, 0x6F, 0x6C, 0x73, 0xc2, 0xae, 0x00};
            string converted = Encoding.UTF8.GetString(x, 0, x.Length);
            Console.WriteLine("Hello World! - {0}",converted);
        }
    }
}

For Little Endian I get:

Hello World! - Microsoft® Build Tools®
6548 6c6c 206f 6f57 6c72 2164 2d20 4d20 6369 6f72 6f73 7466 aec2 4220 6975 646c 5420 6f6f 736c aec2 000a

For Big Endian:

Hello World! - Microsof �� Build Tools��
4865 6c6c 6f20 576f 726c 6421 202d 204d 6963 726f 736f 6620 efbf bdef bfbd 2042 7569 6c64 2054 6f6f 6c73 efbf bdef bfbd 0a00

Now this program didn't lead to an exception but the output is certainly incorrect. There seems to be an attempt at the BOM sequence in big endian output. I am not sure it is required and the bytes could be inserted as is.

Using BlobReader ReadUTF8

Next I tried creating a test case that uses BlobReader and using the same byte sequence as in the exception case to see if I could get it to generate the exception:

using System;
using System.Reflection.Metadata;
using System.Runtime.InteropServices;

namespace HelloWorld
{
    class Program
    {
        static void Main(string[] args)
        {
            string converted;
            Byte[] x = {0xc2, 0xa9, 0x20, 0x4d, 0x69, 0x63, 0x72, 0x6f, 0x73, 0x6f, 0x66, 0x74,
                        0x20, 0x43, 0x6f, 0x72, 0x70, 0x6f, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e,
                        0x2e, 0x20, 0x41, 0x6c, 0x6c, 0x20, 0x72, 0x69, 0x67, 0x68, 0x74, 0x73,
                        0x20, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x65, 0x64, 0x2e, 0x00};
            unsafe {
                fixed (byte *y = x) {
                    IntPtr buffer = Marshal.AllocHGlobal(x.Length);
                    byte* dst = (byte *) buffer;
                    for (int i = 0; i < x.Length; i++) 
                        dst[i] = x[i];
                    BlobReader br = new BlobReader((byte *) buffer, x.Length);
                    converted = br.ReadUTF8(x.Length);
                }
            }
            Console.WriteLine("Hello World! - {0}",converted);
        }
    }
}

On Little Endian:

Hello World! - © Microsoft Corporation. All rights reserved.

On Big Endian I get the exception.

Unhandled Exception:
System.ArgumentException: The output char buffer is too small to contain the decoded characters, encoding 'Unicode (UTF-8)' fallback 'System.Text.DecoderReplacementFallback'. (Parameter 'chars')
   at System.Text.Encoding.ThrowCharsOverflow(DecoderNLS decoder, Boolean nothingDecoded) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs:line 1179

Same Test Case Using a Shorter Sequence

If I reduce the byte sequence to just the first 4 bytes (+ null byte) I also get the exception.

Same Byte Sequence with System.Text.UTF8Encoding.GetString

I also get the exception if I use the first test case with that sequence 4 byte sequence. Here the complete backtrace is:

Unhandled Exception:
System.ArgumentException: The output char buffer is too small to contain the decoded characters, encoding 'Unicode (UTF-8)' fallback 'System.Text.DecoderReplacementFallback'. (Parameter 'chars')
   at System.Text.Encoding.ThrowCharsOverflow(DecoderNLS decoder, Boolean nothingDecoded) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs:line 1179
   at System.Text.Encoding.GetCharsWithFallback(ReadOnlySpan`1 bytes, Int32 originalBytesLength, Span`1 chars, Int32 originalCharsLength, DecoderNLS decoder) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.Internal.cs:line 1272
   at System.Text.UTF8Encoding.GetCharsWithFallback(ReadOnlySpan`1 bytes, Int32 originalBytesLength, Span`1 chars, Int32 originalCharsLength, DecoderNLS decoder) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/UTF8Encoding.cs:line 647
   at System.Text.Encoding.GetCharsWithFallback(Byte* pOriginalBytes, Int32 originalByteCount, Char* pOriginalChars, Int32 originalCharCount, Int32 bytesConsumedSoFar, Int32 charsWrittenSoFar) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.Internal.cs:line 1086
   at System.Text.UTF8Encoding.GetChars(Byte* bytes, Int32 byteCount, Char* chars, Int32 charCount) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/UTF8Encoding.cs:line 553
   at System.String.CreateStringFromEncoding(Byte* bytes, Int32 byteLength, Encoding encoding) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/String.cs:line 540
   at System.Text.UTF8Encoding.GetString(Byte[] bytes, Int32 index, Int32 count) in /home/neale/runtimelab/src/libraries/System.Private.CoreLib/src/System/Text/UTF8Encoding.cs:line 686
   at HelloWorld.Program.Main(String[] args) in /home/neale/HelloWorld/Program.cs:line 14

BlobRead.ReadUTF8 with First Byte Sequence

With the byte sequence in the first test case I don't get the exception but I do get the same garbled output:

Hello World! - Microsof �� Build Tools��

That output is identical to the first example.

I can now compare the --trace output of the test case on both systems to see where things are going awry.

[GrabYourPitchforks]

I pinged the build infra crew offline to see if we can get a big endian machine for testing. Ideally we'd have a CI leg for this. We nominally have very good unit test coverage over this code base.

[nealef]

I can get you an account on one if you'd like. Ubuntu 20.04.

I'm curious as to why what looks like a BOM is not correct but in the code it is. 0xef 0xbb 0xbf is what it should be but 0xef 0xbf 0xbd is what I'm seeing in the output and without the multibyte characters. Also, I'm not sure why it's needed.

Using the mono 6.8 libraries the output of the 1st case is correct on big endian:

Hello World! - Microsoft® Build Tools®

And with the byte sequence of the 2nd case:

Hello World! - © Microsoft Corporation. All rights reserved.
4865 6c6c 6f20 576f 726c 6421 202d 20c2 a920 4d69 6372 6f73 6f66 7420 436f 7270 6f72 6174 696f 6e2e 2041 6c6c 2072 6967 6874 7320 7265 7365 7276 6564 2e0a

No sign of BOM sequence. Mono doesn't have a BlobReader so there's no way to run the 2nd test case with either sequence of bytes.

[GrabYourPitchforks]

I haven't had time to comb through the corelib implementation yet, but here's my suspicion. I suspect what's happening is that there's a discrepancy in the behavior between the UTF-8 validator (which is responsible for counting how many UTF-16 chars would result from a GetString operation) and the UTF-8 transcoder (which is responsible for populating the string). The validator sees the UTF-8 byte sequence [ C2 AE ] as valid and producing the single UTF-16 char '®', but the transcoder sees that byte sequence as invalid and producing the fallback chars "��" (two '\uFFFD' chars). Since these two methods disagree on how large the string should be, an exception is generated during the population step. (If this optimization is instead hit, you won't see an exception, but you'll see a malformed string generated, which explains your earlier sample code.)

Thank you for providing the hello world sample. I'll make time tomorrow to sit down and step through the code with pen + paper. I also have a request out to our build lab team to see if we can get our full unit test battery for this API running on a big endian machine. IMO we have comprehensive unit test coverage here, so getting the battery running on such a machine might flesh out any other latent bugs.

[nealef]

I can send you the entire --trace output that will show all the methods called as well as their parameters and returned values. The trace may be found at http://148.100.42.157/encoding.trc

[nealef]

Have you had a chance to look at this? If not, given the time of year and the recent release of .NET 5, I can appreciate this is a low priority. Using what you described above what methods should I look at to see if I can see where the discrepancy lies.

[nealef]

This patch appears to fix the problem but I've not tested it with a little endian build yet.

diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs
index 6a67e788216..70c4d6bafc6 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.

+using System.Buffers.Binary;
 using System.Diagnostics;
 using System.Numerics;
 using System.Runtime.CompilerServices;
@@ -1719,6 +1720,10 @@ public static unsafe nuint WidenAsciiToUtf16(byte* pAsciiBuffer, char* pUtf16Buf

             // Drain ASCII bytes one at a time.

+            if (!BitConverter.IsLittleEndian)
+            {
+                asciiData = BinaryPrimitives.ReverseEndianness(asciiData);
+            }
             while (((byte)asciiData & 0x80) == 0)
             {
                 pUtf16Buffer[currentOffset] = (char)(byte)asciiData;
diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Helpers.cs b/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Helpers.cs
index ac59f0b5c72..e656f5b55ba 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Helpers.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Helpers.cs
@@ -232,7 +232,7 @@ private static uint ExtractUtf8TwoByteSequenceFromFirstUtf16Char(uint value)
                 // want to return [ ######## ######## 110yyyyy 10xxxxxx ]

                 uint temp = (value >> 16) & 0x3Fu; // [ 00000000 00000000 00000000 00xxxxxx ]
-                value = (value >> 22) & 0x1F00u; // [ 00000000 00000000 000yyyyy 0000000 ]
+                value = (value >> 14) & 0x1F00u; // [ 00000000 00000000 000yyyyy 0000000 ]
                 return value + temp + 0xC080u;
             }
         }

[nealef]

Revised patch set that addresses a couple of other endian issues. The result of these is the following test status:

- 119 complete successfully
- 28 complete, but report at least one failing test case
- 5 do not complete the run (due to assertion failure or crash)
- 1 does not complete due to failure to terminate

bigendian-fixes.patch.txt

[GrabYourPitchforks]

Fixed by #47981.