Repository: ronaldoussoren/macholib

Issue:
Feature: Option for ignoring unknown load commands
**[Original report](https://bitbucket.org/ronaldoussoren/macholib/issue/26) by Mikkel Kamstrup Erlandsen (Bitbucket: [kamikkel](https://bitbucket.org/kamikkel), GitHub: [kamikkel](https://github.com/kamikkel)).**

----------------------------------------

First of all - thanks for a great library! <3

We were hit by the unknown load commands 0x31 and 0x32 that were just recently implemented in macholib. Updating fixed this, of course, but it would be nice to guard against similar issues with an option for the parser to just skip (or print a warning on) unknown load commands.

Background: We use macholib for some simple validation of user-submitted iOS apps. We would like to be able to process newly built apps without redeployment of our toolchain (just recording any warnings it may produce) - disregarding what Apple may come up with in the future.


Discussion:

Patch:
diff --git a/macholib/MachO.py b/macholib/MachO.py
index 3db9520..d4d85f1 100644
--- a/macholib/MachO.py
+++ b/macholib/MachO.py
@@ -92,7 +92,10 @@ def lc_str_value(offset, cmd_info):
 
 class MachO(object):
     """
-    Provides reading/writing the Mach-O header of a specific existing file
+    Provides reading/writing the Mach-O header of a specific existing file.
+
+    If allow_unknown_load_commands is True, allows unknown load commands.
+    Otherwise, raises ValueError if the file contains an unknown load command.
     """
 
     #   filename   - the original filename of this mach-o
@@ -104,7 +107,7 @@ class MachO(object):
     #   low_offset - essentially, the maximum mach-o header size
     #   id_cmd     - the index of my id command, or None
 
-    def __init__(self, filename):
+    def __init__(self, filename, allow_unknown_load_commands=False):
 
         # supports the ObjectGraph protocol
         self.graphident = filename
@@ -114,6 +117,7 @@ class MachO(object):
         # initialized by load
         self.fat = None
         self.headers = []
+        self.allow_unknown_load_commands = allow_unknown_load_commands
         with open(filename, "rb") as fp:
             self.load(fp)
 
@@ -165,7 +169,7 @@ class MachO(object):
             magic, hdr, endian = MH_CIGAM_64, mach_header_64, "<"
         else:
             raise ValueError("Unknown Mach-O header: 0x%08x in %r" % (header, fh))
-        hdr = MachOHeader(self, fh, offset, size, magic, hdr, endian)
+        hdr = MachOHeader(self, fh, offset, size, magic, hdr, endian, self.allow_unknown_load_commands)
         self.headers.append(hdr)
 
     def write(self, f):
@@ -175,7 +179,10 @@ class MachO(object):
 
 class MachOHeader(object):
     """
-    Provides reading/writing the Mach-O header of a specific existing file
+    Provides reading/writing the Mach-O header of a specific existing file.
+
+    If allow_unknown_load_commands is True, allows unknown load commands.
+    Otherwise, raises ValueError if the file contains an unknown load command.
     """
 
     #   filename   - the original filename of this mach-o
@@ -187,7 +194,7 @@ class MachOHeader(object):
     #   low_offset - essentially, the maximum mach-o header size
     #   id_cmd     - the index of my id command, or None
 
-    def __init__(self, parent, fh, offset, size, magic, hdr, endian):
+    def __init__(self, parent, fh, offset, size, magic, hdr, endian, allow_unknown_load_commands=False):
         self.MH_MAGIC = magic
         self.mach_header = hdr
 
@@ -206,6 +213,8 @@ class MachOHeader(object):
         self.filetype = None
         self.headers = []
 
+        self.allow_unknown_load_commands = allow_unknown_load_commands
+
         self.load(fh)
 
     def __repr__(self):
@@ -242,7 +251,16 @@ class MachOHeader(object):
             # read the specific command
             klass = LC_REGISTRY.get(cmd_load.cmd, None)
             if klass is None:
-                raise ValueError("Unknown load command: %d" % (cmd_load.cmd,))
+                if not self.allow_unknown_load_commands:
+                    raise ValueError("Unknown load command: %d" % (cmd_load.cmd,))
+                # No load command in the registry, so append the load command itself
+                # instead of trying to deserialize the data after the header.
+                data_size = cmd_load.cmdsize - sizeof(load_command)
+                cmd_data = fh.read(data_size)
+                cmd.append((cmd_load, cmd_load, cmd_data))
+                read_bytes += cmd_load.cmdsize
+                continue
+
             cmd_cmd = klass.from_fileobj(fh, **kw)
 
             if cmd_load.cmd == LC_ID_DYLIB:


Test patch:
diff --git a/macholib_tests/test_MachO.py b/macholib_tests/test_MachO.py
index c32ea70..ed3d808 100644
--- a/macholib_tests/test_MachO.py
+++ b/macholib_tests/test_MachO.py
@@ -1,17 +1,92 @@
+import contextlib
+import os
+import struct
 import sys
+import tempfile
+import uuid
 
-from macholib import MachO
+from macholib import MachO, mach_o
 
 if sys.version_info[:2] <= (2, 6):
     import unittest2 as unittest
 else:
     import unittest
 
+@contextlib.contextmanager
+def temporary_macho_file(load_commands):
+    struct_mach_header_64_format = '>IIIIIIII'
+    cpu_type_arm64 = 0x100000C
+    cpu_subtype_arm_all = 0x0
+    mh_filetype_execute = 0x2
+    ncmds = len(load_commands)
+    sizeofcmds = sum([len(lc) for lc in load_commands])
+    mach_header = struct.pack(struct_mach_header_64_format, mach_o.MH_MAGIC_64,
+                              cpu_type_arm64, cpu_subtype_arm_all,
+                              mh_filetype_execute, ncmds, sizeofcmds, 0, 0)
+    with tempfile.NamedTemporaryFile(delete=False) as macho_file:
+        macho_file.write(mach_header)
+        for lc in load_commands:
+            macho_file.write(lc)
+        # Close the file so it can be re-opened on Windows.
+        macho_file.close()
+        yield macho_file.name
+        os.unlink(macho_file.name)
+
+
+def lc_uuid(macho_uuid):
+    lc_uuid_format = '>II16s'
+    lc_uuid_size = struct.calcsize(lc_uuid_format)
+    return struct.pack(lc_uuid_format, mach_o.LC_UUID, lc_uuid_size, macho_uuid.bytes)
+
+
+def lc_unknown():
+    lc_unknown_format = '>III'
+    lc_unknown = 0x707A11ED  # Made-up load command. Hopefully never used.
+    lc_unknown_size = struct.calcsize(lc_unknown_format)
+    lc_unknown_value = 42  # Random value
+    return struct.pack(lc_unknown_format, lc_unknown, lc_unknown_size, lc_unknown_value)
+
 
 class TestMachO(unittest.TestCase):
-    @unittest.expectedFailure
-    def test_missing(self):
-        self.fail("tests are missing")
+    def test_known_load_command_should_succeed(self):
+        macho_uuid = uuid.UUID('6894C0AE-C8B7-4E0B-A529-30BBEBA3703B')
+        with temporary_macho_file([lc_uuid(macho_uuid)]) as macho_filename:
+            macho = MachO.MachO(macho_filename, allow_unknown_load_commands=True)
+            self.assertEqual(len(macho.headers), 1)
+            self.assertEqual(len(macho.headers[0].commands), 1)
+            load_command, command, _ = macho.headers[0].commands[0]
+            self.assertEqual(load_command.cmd, mach_o.LC_UUID)
+            self.assertEqual(uuid.UUID(bytes=command.uuid), macho_uuid)
+
+    def test_unknown_load_command_should_fail(self):
+        with temporary_macho_file([lc_unknown()]) as macho_filename:
+            with self.assertRaises(ValueError) as assert_context:
+                MachO.MachO(macho_filename)
+
+    def test_unknown_load_command_should_succeed_with_flag(self):
+        with temporary_macho_file([lc_unknown()]) as macho_filename:
+            macho = MachO.MachO(macho_filename, allow_unknown_load_commands=True)
+            self.assertEqual(len(macho.headers), 1)
+            self.assertEqual(len(macho.headers[0].commands), 1)
+            load_command, command, data = macho.headers[0].commands[0]
+            self.assertEqual(load_command.cmd, 0x707A11ED)
+            self.assertIsInstance(command, mach_o.load_command)
+            self.assertEqual(struct.unpack('>I', data), (42,))
+
+
+    def test_mix_of_known_and_unknown_load_commands_should_allow_unknown_with_flag(self):
+        macho_uuid = uuid.UUID('6894C0AE-C8B7-4E0B-A529-30BBEBA3703B')
+        with temporary_macho_file([lc_unknown(), lc_uuid(macho_uuid)]) as macho_filename:
+            macho = MachO.MachO(macho_filename, allow_unknown_load_commands=True)
+            self.assertEqual(len(macho.headers), 1)
+            self.assertEqual(len(macho.headers[0].commands), 2)
+            load_command, command, data = macho.headers[0].commands[0]
+            self.assertEqual(load_command.cmd, 0x707A11ED)
+            self.assertIsInstance(command, mach_o.load_command)
+            self.assertEqual(struct.unpack('>I', data), (42,))
+            load_command, command, _ = macho.headers[0].commands[1]
+            self.assertEqual(load_command.cmd, mach_o.LC_UUID)
+            self.assertEqual(uuid.UUID(bytes=command.uuid), macho_uuid)
 
 
 if __name__ == "__main__":