Fix Mach-O boundary validation

Author: xoofx

src/LibObjectFile.Tests/MachO/MachOSigningTests.cs

@@ -254,6 +254,27 @@ public void RejectsWhatCannotBeSigned()
         ByteArrayAssert.AreEqual(before, WriteToArray(file), "a failed signing changed the image");
     }
 
+    [TestMethod]
+    public void RejectsSigningOffsetsThatCannotBeAligned()
+    {
+        var file = LoadMachO("unixthread_i386");
+        var trailing = file.Content[^1];
+        trailing.Position = uint.MaxValue - trailing.Size;
+        var commandCount = file.LoadCommands.Count;
+
+        var exception = Assert.ThrowsExactly<InvalidOperationException>(() => file.AdHocSign("swkotor"));
+        StringAssert.Contains(exception.Message, "cannot be aligned");
+        Assert.AreEqual(commandCount, file.LoadCommands.Count, "failed signing left its load command behind");
+    }
+
+    [TestMethod]
+    public void ComputesCodeSlotsWithoutOverflow()
+    {
+        var builder = new MachOAdHocSignatureBuilder("swkotor") { CodeLimit = uint.MaxValue };
+
+        Assert.AreEqual(1_048_576u, builder.CodeSlotCount);
+    }
+
     private static Span<byte> FindCodeDirectory(byte[] image, uint signatureOffset)
         => FindBlob(image, signatureOffset, CodeDirectorySlot);
 

src/LibObjectFile.Tests/MachO/MachOSimpleTests.cs

@@ -3,6 +3,7 @@
 // See the license.txt file in the project root for more information.
 
 using System;
+using System.Buffers.Binary;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -497,6 +498,13 @@ public void ReadsAndWritesAUniversalBinary()
         tooFar.Slices[1].FileOffset = 0x1_0000_0000;
         tooFar.Slices[1].Size = 0x10;
         Assert.IsTrue(tooFar.Verify().Messages.Any(m => m.Id == DiagnosticId.MACHO_ERR_ValueTooLargeFor32Bit));
+
+        // The table is what the loader uses to select a slice, so it has to describe the image
+        // that will actually be found there.
+        var mismatched = MachOFatFile.Read(new MemoryStream(original));
+        mismatched.Slices[0].CpuType = MachOCpuType.X86;
+        Assert.IsFalse(mismatched.TryWrite(new MemoryStream(), out var mismatchDiagnostics));
+        Assert.IsTrue(mismatchDiagnostics.Messages.Any(m => m.Id == DiagnosticId.MACHO_ERR_FatSliceArchitectureMismatch), string.Join("; ", mismatchDiagnostics.Messages));
     }
 
     /// <summary>
@@ -646,5 +654,35 @@ public void ReportsRatherThanThrowsOnUnreadableInput()
         BitConverter.GetBytes(0u).Reverse().ToArray().CopyTo(fat, MachOFatFile.HeaderSize + 12);
         Assert.IsFalse(MachOFatFile.TryRead(new MemoryStream(fat), out _, out var emptySlice));
         Assert.IsTrue(emptySlice.Messages.Any(m => m.Id == DiagnosticId.MACHO_ERR_InvalidFatArchRange), string.Join("; ", emptySlice.Messages));
+
+        // FAT64 offsets and sizes are unsigned 64-bit fields. An end that wraps must be rejected
+        // before either value is narrowed for SubStream.
+        var wrappingFatRange = new byte[MachOFatFile.HeaderSize + MachOFatFile.GetSliceEntrySize(true)];
+        BinaryPrimitives.WriteUInt32BigEndian(wrappingFatRange, MachOMagic.FatMagic64);
+        BinaryPrimitives.WriteUInt32BigEndian(wrappingFatRange.AsSpan(4), 1);
+        BinaryPrimitives.WriteUInt64BigEndian(wrappingFatRange.AsSpan(MachOFatFile.HeaderSize + 8), ulong.MaxValue - 15);
+        BinaryPrimitives.WriteUInt64BigEndian(wrappingFatRange.AsSpan(MachOFatFile.HeaderSize + 16), MachOFile.MinHeaderSize);
+        Assert.IsFalse(MachOFatFile.TryRead(new MemoryStream(wrappingFatRange), out _, out var wrappingFatSlice));
+        Assert.IsTrue(wrappingFatSlice.Messages.Any(m => m.Id == DiagnosticId.MACHO_ERR_InvalidFatArchRange), string.Join("; ", wrappingFatSlice.Messages));
+
+        // The same overflow is possible in a 64-bit section size. Build the smallest image with
+        // one LC_SEGMENT_64 and one section whose declared end wraps back into the file.
+        const int machHeaderSize = 32;
+        const int segmentCommandSize = 72;
+        const int sectionSize = 80;
+        var wrappingSectionRange = new byte[machHeaderSize + segmentCommandSize + sectionSize];
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange, MachOMagic.Magic64);
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(4), (uint)MachOCpuType.X86_64);
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(12), (uint)MachOFileType.Object);
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(16), 1);
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(20), segmentCommandSize + sectionSize);
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(machHeaderSize), (uint)MachOLoadCommandType.Segment64);
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(machHeaderSize + 4), segmentCommandSize + sectionSize);
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(machHeaderSize + 64), 1);
+        var sectionOffset = machHeaderSize + segmentCommandSize;
+        BinaryPrimitives.WriteUInt64LittleEndian(wrappingSectionRange.AsSpan(sectionOffset + 40), unchecked(ulong.MaxValue - (ulong)wrappingSectionRange.Length + 11));
+        BinaryPrimitives.WriteUInt32LittleEndian(wrappingSectionRange.AsSpan(sectionOffset + 48), (uint)wrappingSectionRange.Length);
+        Assert.IsFalse(MachOFile.TryRead(new MemoryStream(wrappingSectionRange), out _, out var wrappingSection));
+        Assert.IsTrue(wrappingSection.Messages.Any(m => m.Id == DiagnosticId.MACHO_ERR_InvalidContentFileRange), string.Join("; ", wrappingSection.Messages));
     }
 }

src/LibObjectFile/Diagnostics/DiagnosticId.cs

@@ -241,4 +241,5 @@ public enum DiagnosticId
     MACHO_ERR_MissingFatSliceImage = 5023,
     MACHO_ERR_UnexpectedEndOfStream = 5024,
     MACHO_ERR_UnexpectedArchive = 5025,
-}
+    MACHO_ERR_FatSliceArchitectureMismatch = 5026,
+}

src/LibObjectFile/MachO/CodeSign/MachOAdHocSignatureBuilder.cs

@@ -73,7 +73,7 @@ public MachOAdHocSignatureBuilder(string identifier)
     /// <summary>
     /// Gets the number of page hashes the code directory will hold for the current <see cref="CodeLimit"/>.
     /// </summary>
-    public uint CodeSlotCount => (CodeLimit + PageSize - 1) / PageSize;
+    public uint CodeSlotCount => (uint)(((ulong)CodeLimit + PageSize - 1) / PageSize);
 
     /// <summary>
     /// Gets the total size of the signature for the current <see cref="CodeLimit"/> and identifier.

src/LibObjectFile/MachO/MachOFatFile.Verify.cs

@@ -88,6 +88,13 @@ public void Verify(DiagnosticBag diagnostics)
                     $"Slice {i} repeats the architecture {slice.CpuType} (subtype 0x{slice.CpuSubType:X}) of an earlier slice");
             }
 
+            if (slice.CpuType != slice.File.CpuType || slice.CpuSubType != slice.File.CpuSubType)
+            {
+                diagnostics.Error(
+                    DiagnosticId.MACHO_ERR_FatSliceArchitectureMismatch,
+                    $"Slice {i} is recorded as {slice.CpuType} (subtype 0x{slice.CpuSubType:X}) but contains {slice.File.CpuType} (subtype 0x{slice.File.CpuSubType:X})");
+            }
+
             ranges.Add((slice.FileOffset, slice.FileOffset + slice.Size, i));
             slice.File.Verify(diagnostics);
         }

src/LibObjectFile/MachO/MachOFatFile.cs

@@ -112,6 +112,7 @@ public static bool TryRead(Stream stream, [NotNullWhen(true)] out MachOFatFile?
         diagnostics = bag;
 
         var basePosition = stream.Position;
+        var availableLength = (ulong)(stream.Length - basePosition);
         Span<byte> header = stackalloc byte[HeaderSize];
         stream.Position = basePosition;
         if (stream.Read(header) != HeaderSize)
@@ -188,9 +189,11 @@ public static bool TryRead(Stream stream, [NotNullWhen(true)] out MachOFatFile?
                 return false;
             }
 
-            if (slice.FileOffset + slice.Size > (ulong)stream.Length)
+            // Offsets are relative to the universal binary, not the underlying stream. Compare
+            // by subtraction so FAT64 values cannot wrap and then be narrowed to invalid longs.
+            if (slice.FileOffset > availableLength || slice.Size > availableLength - slice.FileOffset)
             {
-                bag.Error(DiagnosticId.MACHO_ERR_InvalidFatArchRange, $"Slice {i} spans [0x{slice.FileOffset:X}, 0x{slice.FileOffset + slice.Size:X}) which extends past the end of the file");
+                bag.Error(DiagnosticId.MACHO_ERR_InvalidFatArchRange, $"Slice {i} starts at 0x{slice.FileOffset:X} and spans 0x{slice.Size:X} bytes, which extends past the end of the file");
                 file = null;
                 return false;
             }

src/LibObjectFile/MachO/MachOFile.Read.cs

@@ -372,7 +372,9 @@ void Add(ulong offset, ulong size, MachOSection? section = null)
             // An offset of zero means the data is absent rather than at the start of the file.
             if (offset < commandsEnd || size == 0) return;
 
-            if (offset + size > fileLength)
+            // Compare by subtraction so an untrusted 64-bit size cannot wrap the end back into
+            // the file and reach the stream reader as a very large allocation.
+            if (offset > fileLength || size > fileLength - offset)
             {
                 reader.Diagnostics.Error(
                     DiagnosticId.MACHO_ERR_InvalidContentFileRange,

src/LibObjectFile/MachO/MachOFile.Sign.cs

@@ -94,7 +94,12 @@ public void AdHocSign(string identifier)
                 throw new InvalidOperationException($"The image is 0x{contentEnd:X} bytes, too large for the 32-bit offset LC_CODE_SIGNATURE records.");
             }
 
-            var signatureOffset = AlignHelper.AlignUp((uint)contentEnd, (uint)MachOCodeSignatureConstants.SignatureAlignment);
+            var alignedSignatureOffset = AlignHelper.AlignUp(contentEnd, (ulong)MachOCodeSignatureConstants.SignatureAlignment);
+            if (alignedSignatureOffset > uint.MaxValue)
+            {
+                throw new InvalidOperationException($"The image ends at 0x{contentEnd:X}, which cannot be aligned for the 32-bit offset LC_CODE_SIGNATURE records.");
+            }
+            var signatureOffset = (uint)alignedSignatureOffset;
 
             var builder = new MachOAdHocSignatureBuilder(identifier)
             {
@@ -108,7 +113,7 @@ public void AdHocSign(string identifier)
             command.DataOffset = signatureOffset;
             command.DataSize = signatureSize;
 
-            linkEdit.FileSize = signatureOffset + signatureSize - linkEdit.FileOffset;
+            linkEdit.FileSize = (ulong)signatureOffset + signatureSize - linkEdit.FileOffset;
 
             // The signature is far larger than the slack the linker left, so __LINKEDIT grows and
             // has to be rounded to a segment boundary the way codesign rounds it. Without this it