From 655835536b45bf4485498559604a57f32d6e52b2 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 19 Jun 2026 18:17:48 +0200 Subject: [PATCH 1/4] Remove unsafe code from System.Reflection.Metadata PE/blob writers Replace fixed-pointer writes with BinaryPrimitives in SubstituteTemplateParameters and the PE checksum walk, and drop the IEnumerable/iterator allocation in CalculateChecksum by iterating the struct enumerator directly. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Metadata/TypeSystem/Handles.TypeSystem.cs | 8 +- .../PortableExecutable/PEBuilder.cs | 94 ++++++++----------- 2 files changed, 44 insertions(+), 58 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeSystem/Handles.TypeSystem.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeSystem/Handles.TypeSystem.cs index 5d610fed99e949..e2de1c221e8d19 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeSystem/Handles.TypeSystem.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeSystem/Handles.TypeSystem.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.Diagnostics.CodeAnalysis; using System.Reflection.Metadata.Ecma335; @@ -2591,14 +2592,11 @@ internal static BlobHandle FromVirtualIndex(VirtualIndex virtualIndex, ushort vi internal const int TemplateParameterOffset_AttributeUsageTarget = 2; - internal unsafe void SubstituteTemplateParameters(byte[] blob) + internal void SubstituteTemplateParameters(byte[] blob) { Debug.Assert(blob.Length >= TemplateParameterOffset_AttributeUsageTarget + 4); - fixed (byte* ptr = &blob[TemplateParameterOffset_AttributeUsageTarget]) - { - *((uint*)ptr) = VirtualValue; - } + BinaryPrimitives.WriteUInt32LittleEndian(blob.AsSpan(TemplateParameterOffset_AttributeUsageTarget), VirtualValue); } public static implicit operator Handle(BlobHandle handle) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs index 8389ff7359c1b0..2daf0c6bd7d292 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.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.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; @@ -467,23 +468,6 @@ internal static IEnumerable GetContentToSign(BlobBuilder peImage, int peHe internal static Blob GetPrefixBlob(Blob container, Blob blob) => new Blob(container.Buffer, container.Start, blob.Start - container.Start); internal static Blob GetSuffixBlob(Blob container, Blob blob) => new Blob(container.Buffer, blob.Start + blob.Length, container.Start + container.Length - blob.Start - blob.Length); - // internal for testing - internal static IEnumerable GetContentToChecksum(BlobBuilder peImage, Blob checksumFixup) - { - foreach (var blob in peImage.GetBlobs()) - { - if (blob.Buffer == checksumFixup.Buffer) - { - yield return GetPrefixBlob(blob, checksumFixup); - yield return GetSuffixBlob(blob, checksumFixup); - } - else - { - yield return blob; - } - } - } - internal void Sign(BlobBuilder peImage, Blob strongNameSignatureFixup, Func, byte[]> signatureProvider) { Debug.Assert(peImage != null); @@ -508,57 +492,61 @@ internal void Sign(BlobBuilder peImage, Blob strongNameSignatureFixup, Func blobs) { uint checksum = 0; int pendingByte = -1; - foreach (var blob in blobs) + foreach (Blob blob in peImage.GetBlobs()) { - var segment = blob.GetBytes(); - fixed (byte* arrayPtr = segment.Array) + if (blob.Buffer == checksumFixup.Buffer) + { + // The checksum field itself is excluded, so checksum the content before and + // after the fixup as two separate segments. + AddToChecksum(GetPrefixBlob(blob, checksumFixup).GetBytes(), ref checksum, ref pendingByte); + AddToChecksum(GetSuffixBlob(blob, checksumFixup).GetBytes(), ref checksum, ref pendingByte); + } + else { - Debug.Assert(segment.Count > 0); + AddToChecksum(blob.GetBytes(), ref checksum, ref pendingByte); + } + } - byte* ptr = arrayPtr + segment.Offset; - byte* end = ptr + segment.Count; + if (pendingByte >= 0) + { + checksum = AggregateChecksum(checksum, (ushort)pendingByte); + } - if (pendingByte >= 0) - { - // little-endian encoding: - checksum = AggregateChecksum(checksum, (ushort)(*ptr << 8 | pendingByte)); - ptr++; - } + return checksum + (uint)peImage.Count; + } - if ((end - ptr) % 2 != 0) - { - end--; - pendingByte = *end; - } - else - { - pendingByte = -1; - } + private static void AddToChecksum(ArraySegment bytes, ref uint checksum, ref int pendingByte) + { + ReadOnlySpan segment = bytes.AsSpan(); + Debug.Assert(segment.Length > 0); - while (ptr < end) - { - // little-endian encoding: - checksum = AggregateChecksum(checksum, (ushort)(ptr[1] << 8 | ptr[0])); - ptr += sizeof(ushort); - } - } + if (pendingByte >= 0) + { + // little-endian encoding: + checksum = AggregateChecksum(checksum, (ushort)(segment[0] << 8 | pendingByte)); + segment = segment.Slice(1); } - if (pendingByte >= 0) + if (segment.Length % 2 != 0) { - checksum = AggregateChecksum(checksum, (ushort)pendingByte); + pendingByte = segment[segment.Length - 1]; + segment = segment.Slice(0, segment.Length - 1); + } + else + { + pendingByte = -1; } - return checksum; + while (segment.Length >= sizeof(ushort)) + { + // little-endian encoding: + checksum = AggregateChecksum(checksum, BinaryPrimitives.ReadUInt16LittleEndian(segment)); + segment = segment.Slice(sizeof(ushort)); + } } private static uint AggregateChecksum(uint checksum, ushort value) From 2f1a085f757d5494d9616ae689990b79436fc7c1 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 19 Jun 2026 18:24:07 +0200 Subject: [PATCH 2/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/System/Reflection/PortableExecutable/PEBuilder.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs index 2daf0c6bd7d292..1c6f6f6162f884 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs @@ -522,7 +522,10 @@ internal static uint CalculateChecksum(BlobBuilder peImage, Blob checksumFixup) private static void AddToChecksum(ArraySegment bytes, ref uint checksum, ref int pendingByte) { ReadOnlySpan segment = bytes.AsSpan(); - Debug.Assert(segment.Length > 0); + if (segment.IsEmpty) + { + return; + } if (pendingByte >= 0) { From 65a537c7ab0e97a8cbc570220eb65158c1ba7723 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 20 Jun 2026 04:23:20 +0200 Subject: [PATCH 3/4] Update PEBuilder.cs --- .../src/System/Reflection/PortableExecutable/PEBuilder.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs index 1c6f6f6162f884..4e9bd422e370c1 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs @@ -519,9 +519,8 @@ internal static uint CalculateChecksum(BlobBuilder peImage, Blob checksumFixup) return checksum + (uint)peImage.Count; } - private static void AddToChecksum(ArraySegment bytes, ref uint checksum, ref int pendingByte) + private static void AddToChecksum(ReadOnlySpan segment, ref uint checksum, ref int pendingByte) { - ReadOnlySpan segment = bytes.AsSpan(); if (segment.IsEmpty) { return; From a799b279af6147923d83d846549e5e4a850d891f Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 26 Jun 2026 02:17:36 +0200 Subject: [PATCH 4/4] Simplify trailing-byte handling in PE checksum walk Move the odd trailing-byte carry below the pair loop per review feedback, so it reduces to a single 'pendingByte = segment.IsEmpty ? -1 : segment[0];' assignment. Behavior is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Reflection/PortableExecutable/PEBuilder.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs index 4e9bd422e370c1..73107af667990a 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEBuilder.cs @@ -533,22 +533,15 @@ private static void AddToChecksum(ReadOnlySpan segment, ref uint checksum, segment = segment.Slice(1); } - if (segment.Length % 2 != 0) - { - pendingByte = segment[segment.Length - 1]; - segment = segment.Slice(0, segment.Length - 1); - } - else - { - pendingByte = -1; - } - while (segment.Length >= sizeof(ushort)) { // little-endian encoding: checksum = AggregateChecksum(checksum, BinaryPrimitives.ReadUInt16LittleEndian(segment)); segment = segment.Slice(sizeof(ushort)); } + + // Carry a trailing odd byte over to the next segment. + pendingByte = segment.IsEmpty ? -1 : segment[0]; } private static uint AggregateChecksum(uint checksum, ushort value)