Merge pull request #1030 from alebcay/fat-arch-alignment

Validate FatArch alignment and reject truncated Fat architecture records

Author: p-linnane

lib/macho/exceptions.rb

@@ -238,6 +238,15 @@ def initialize(offset)
     end
   end
 
+  # Raised when a fat architecture record specifies an invalid alignment value.
+  # @param align [Integer] the invalid alignment exponent
+  class FatArchAlignmentError < NotAMachOError
+    # @param align [Integer] the invalid alignment exponent
+    def initialize(align)
+      super("Invalid fat architecture alignment: 2**#{align} exceeds maximum supported value")
+    end
+  end
+
   # Raised when attempting to parse a compressed Mach-O without explicitly
   # requesting decompression.
   class CompressedMachOError < MachOError

lib/macho/fat_file.rb

@@ -371,6 +371,7 @@ def populate_fat_header
 
     # Obtain an array of fat architectures from raw file data.
     # @return [Array<Headers::FatArch>] an array of fat architectures
+    # @raise [TruncatedFileError] if the file is too small to contain all fat architectures
     # @api private
     def populate_fat_archs
       archs = []
@@ -380,7 +381,14 @@ def populate_fat_archs
       fa_len   = fa_klass.bytesize
 
       header.nfat_arch.times do |i|
-        archs << fa_klass.new_from_bin(:big, @raw_data[fa_off + (fa_len * i), fa_len])
+        arch_off = fa_off + (fa_len * i)
+        arch_bin = @raw_data[arch_off, fa_len]
+        raise TruncatedFileError if arch_bin.nil? || arch_bin.bytesize < fa_len
+
+        arch = fa_klass.new_from_bin(:big, arch_bin)
+        raise FatArchAlignmentError, arch.align if arch.align > Headers::MAX_FAT_ARCH_ALIGN
+
+        archs << arch
       end
 
       archs

lib/macho/headers.rb

@@ -537,6 +537,12 @@ def to_h
       end
     end
 
+    # Maximum supported fat architecture alignment exponent.
+    # The fat architecture `align` field stores an exponent N where the actual
+    # alignment is 2**N. This limit matches MAXSECTALIGN from cctools and
+    # prevents excessive padding allocation during Fat file reconstruction.
+    MAX_FAT_ARCH_ALIGN = 15
+
     # 32-bit fat binary header architecture structure. A 32-bit fat Mach-O has one or more of
     #  these, indicating one or more internal Mach-O blobs.
     # @note "32-bit" indicates the fact that this structure stores 32-bit offsets, not that the

test/test_code_signing.rb

@@ -248,6 +248,31 @@ def test_rejects_duplicate_signature_commands
     end
   end
 
+  def test_rejects_invalid_fat_alignment_before_signing
+    # Build a valid Fat file and then corrupt the alignment field to exceed MAX_FAT_ARCH_ALIGN
+    machos = SINGLE_ARCHES.map { |a| MachO::MachOFile.new(fixture(a, "hello.bin")) }
+    fat = MachO::FatFile.new_from_machos(*machos)
+    fat_bin = fat.serialize
+
+    # FAT_MAGIC (32-bit) FatArch structure: align at offset 8 + 16 = 24
+    fat_header_size = MachO::Headers::FatHeader.bytesize
+    align_offset = fat_header_size + 16
+
+    # Set alignment to MAX_FAT_ARCH_ALIGN + 1 (16)
+    fat_bin[align_offset, 4] = [MachO::Headers::MAX_FAT_ARCH_ALIGN + 1].pack("N")
+
+    tempfile_with_data("hello", fat_bin) do |file|
+      original = File.binread(file.path)
+
+      assert_raises MachO::CodeSigningError do
+        MachO.codesign!(file.path)
+      end
+
+      # File should not be modified when parsing fails
+      assert_equal original, File.binread(file.path)
+    end
+  end
+
   private
 
   def entitlement_blob

test/test_fat.rb

@@ -56,6 +56,81 @@ def test_zero_arch_file
     end
   end
 
+  def test_invalid_fat_arch_alignment_32
+    # Build a valid Fat file and then corrupt the alignment field to exceed MAX_FAT_ARCH_ALIGN
+    machos = SINGLE_ARCHES.map { |a| MachO::MachOFile.new(fixture(a, "hello.bin")) }
+    fat = MachO::FatFile.new_from_machos(*machos)
+    fat_bin = fat.serialize
+
+    # FAT_MAGIC (32-bit) FatArch structure: cputype(4) + cpusubtype(4) + offset(4) + size(4) + align(4) = 20 bytes
+    # First FatArch starts at offset 8 (FatHeader is 8 bytes)
+    # align is at offset 8 + 16 = 24
+    fat_header_size = MachO::Headers::FatHeader.bytesize
+    align_offset = fat_header_size + 16 # cputype(4) + cpusubtype(4) + offset(4) + size(4)
+
+    # Set alignment to MAX_FAT_ARCH_ALIGN + 1 (16)
+    fat_bin[align_offset, 4] = [MachO::Headers::MAX_FAT_ARCH_ALIGN + 1].pack("N")
+
+    assert_raises MachO::FatArchAlignmentError do
+      MachO::FatFile.new_from_bin(fat_bin)
+    end
+  end
+
+  def test_invalid_fat_arch_alignment_64
+    # Build a valid Fat64 file and then corrupt the alignment field to exceed MAX_FAT_ARCH_ALIGN
+    machos = SINGLE_ARCHES.map { |a| MachO::MachOFile.new(fixture(a, "hello.bin")) }
+    fat = MachO::FatFile.new_from_machos(*machos, :fat64 => true)
+    fat_bin = fat.serialize
+
+    # FAT_MAGIC_64 (64-bit) FatArch64 structure: cputype(4) + cpusubtype(4) + offset(8) + size(8) + align(4) + reserved(4) = 32 bytes
+    # First FatArch64 starts at offset 8 (FatHeader is 8 bytes)
+    # align is at offset 8 + 24 = 32
+    fat_header_size = MachO::Headers::FatHeader.bytesize
+    align_offset = fat_header_size + 24 # cputype(4) + cpusubtype(4) + offset(8) + size(8)
+
+    # Set alignment to MAX_FAT_ARCH_ALIGN + 1 (16)
+    fat_bin[align_offset, 4] = [MachO::Headers::MAX_FAT_ARCH_ALIGN + 1].pack("N")
+
+    assert_raises MachO::FatArchAlignmentError do
+      MachO::FatFile.new_from_bin(fat_bin)
+    end
+  end
+
+  def test_max_valid_fat_arch_alignment_accepted
+    # Build a valid Fat file and set alignment to exactly MAX_FAT_ARCH_ALIGN
+    machos = SINGLE_ARCHES.map { |a| MachO::MachOFile.new(fixture(a, "hello.bin")) }
+    fat = MachO::FatFile.new_from_machos(*machos)
+    fat_bin = fat.serialize
+
+    fat_header_size = MachO::Headers::FatHeader.bytesize
+    align_offset = fat_header_size + 16
+
+    # Set alignment to MAX_FAT_ARCH_ALIGN (15) - should be accepted
+    fat_bin[align_offset, 4] = [MachO::Headers::MAX_FAT_ARCH_ALIGN].pack("N")
+
+    # Should parse without raising
+    parsed = MachO::FatFile.new_from_bin(fat_bin)
+    assert_equal MachO::Headers::MAX_FAT_ARCH_ALIGN, parsed.fat_archs.first.align
+  end
+
+  def test_truncated_fat_arch_32
+    # FAT_MAGIC + nfat_arch=1 + FatArch with only 16 bytes (cputype, cpusubtype, offset, size) missing align (4 bytes)
+    # Total = 8 + 16 = 24 bytes instead of 28 bytes
+    bin = [MachO::Headers::FAT_MAGIC, 1, MachO::Headers::CPU_TYPE_I386, 3, 24, 0].pack("N6")
+    assert_raises MachO::TruncatedFileError do
+      MachO::FatFile.new_from_bin(bin)
+    end
+  end
+
+  def test_truncated_fat_arch_64
+    # FAT_MAGIC_64 + nfat_arch=1 + FatArch64 with 24 bytes (cputype, cpusubtype, offset(8), size(8)) missing align(4) + reserved(4)
+    # Total = 8 + 24 = 32 bytes instead of 40 bytes
+    bin = [MachO::Headers::FAT_MAGIC_64, 1].pack("N2") + [MachO::Headers::CPU_TYPE_I386, 3, 24, 0].pack("N2Q>2")
+    assert_raises MachO::TruncatedFileError do
+      MachO::FatFile.new_from_bin(bin)
+    end
+  end
+
   def test_mismatch_cpu_arch_file
     assert_raises MachO::CPUTypeMismatchError do
       MachO::FatFile.new("test/bin/llvm/macho-invalid-fat_cputype")