From 60aab61a3b9f5d7ff85c9b385709b1282c2e7c83 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 18 Feb 2016 16:17:42 -0500 Subject: [PATCH 1/2] Optimization: Replace control-flow-by-exception with magic testing and incremental reads. Instead of attempting to open a file with MachOFile.new and catching a FatBinaryError to attempt FatFile.new, read a few bytes and do some magic/sanity tests. Once sanity is established, instantiate either a MachOFile or a FatFile (or error out with a MagicError). Additionally, stop truncated files (< 4 bytes) ahead of time with a TruncationError. --- lib/macho/fat_file.rb | 3 +++ lib/macho/macho_file.rb | 3 +++ lib/macho/open.rb | 18 ++++++++++++++---- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/macho/fat_file.rb b/lib/macho/fat_file.rb index 0b25483a9..5bf217148 100644 --- a/lib/macho/fat_file.rb +++ b/lib/macho/fat_file.rb @@ -4,6 +4,9 @@ module MachO # @see https://en.wikipedia.org/wiki/Mach-O#Multi-architecture_binaries # @see MachO::MachOFile class FatFile + # @return [String] the filename loaded from, or nil if loaded from a binary string + attr_accessor :filename + # @return [MachO::FatHeader] the file's header attr_reader :header diff --git a/lib/macho/macho_file.rb b/lib/macho/macho_file.rb index dfdf4ae2c..fe577d5cd 100644 --- a/lib/macho/macho_file.rb +++ b/lib/macho/macho_file.rb @@ -5,6 +5,9 @@ module MachO # @see https://en.wikipedia.org/wiki/Mach-O # @see MachO::FatFile class MachOFile + # @return [String] the filename loaded from, or nil if loaded from a binary string + attr_accessor :filename + # @return [MachO::MachHeader] if the Mach-O is 32-bit # @return [MachO::MachHeader64] if the Mach-O is 64-bit attr_reader :header diff --git a/lib/macho/open.rb b/lib/macho/open.rb index 2c5fc0469..8816554c8 100644 --- a/lib/macho/open.rb +++ b/lib/macho/open.rb @@ -3,14 +3,24 @@ module MachO # @param filename [String] the file being opened # @return [MachO::MachOFile] if the file is a Mach-O # @return [MachO::FatFile] if the file is a Fat file + # @raise [ArgumentError] if the given file does not exist + # @raise [MachO::TruncatedFileError] if the file is too small to have a valid header + # @raise [MachO::MagicError] if the file's magic is not valid Mach-O magic def self.open(filename) + raise ArgumentError.new("#{filename}: no such file") unless File.file?(filename) + raise TruncatedFileError.new unless File.stat(filename).size >= 4 + + magic = File.open(filename, "rb") { |f| f.read(4) }.unpack("N").first + # open file and test magic instead of using exceptions for control? - begin - file = MachOFile.new(filename) - rescue FatBinaryError + if MachO.fat_magic?(magic) file = FatFile.new(filename) + elsif MachO.magic?(magic) + file = MachOFile.new(filename) + else + raise MagicError.new(magic) end file end -end \ No newline at end of file +end From ee8906cc025ada8b4c1ba71206fdb4dcf53aaafc Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 18 Feb 2016 16:52:19 -0500 Subject: [PATCH 2/2] Testing: Add tests for new MachO.open Ensure that MachO.open raises all expected exceptions. --- test/test_open.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/test_open.rb b/test/test_open.rb index 6b7c5a42d..9e7887f6f 100644 --- a/test/test_open.rb +++ b/test/test_open.rb @@ -5,6 +5,32 @@ class MachOOpenTest < Minitest::Test include Helpers + def test_nonexistent_file + assert_raises ArgumentError do + MachO.open("/this/is/a/file/that/cannot/possibly/exist") + end + end + + # MachO.open has slightly looser qualifications for truncation than + # either MachOFile.new or FatFile.new - it just makes sure that there are + # enough magic bytes to read, and lets the actual parser raise a + # TruncationError later on if required. + def test_truncated_file + tempfile_with_data("truncated_file", "\x00\x00") do |truncated_file| + assert_raises MachO::TruncatedFileError do + MachO.open(truncated_file.path) + end + end + end + + def test_bad_magic + tempfile_with_data("junk_file", "\xFF\xFF\xFF\xFF") do |junk_file| + assert_raises MachO::MagicError do + MachO.open(junk_file.path) + end + end + end + def test_open file = MachO.open("test/bin/libhello.dylib")