Make the Mach-O size checking a little more robust

It's not necessary to check at every point if we know the minimum file
size: it must contain at least the header, one segment (__TEXT) and one
section (qtmetadata). Most files have more than one segment and more
than one loader command, so this check does not mean we can eliminate
the checks further down.

Also be more resilient against corruptions in the header data: check not
only the additions, but the values themselves. For example, an offset +
size addition could be smaller than the file size when the addition
overflows in 32-bit. Another thing is that the cmdsize fields could be
corrupt too.

Change-Id: I7968a769c1cbe9150270c91823cafc4f8f833876
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

4 files changed, 247 insertions, 23 deletions

diff --git a/src/corelib/plugin/qmachparser.cpp b/src/corelib/plugin/qmachparser.cpp
index b9a56a403c..5152a92d1d 100644
--- a/src/corelib/plugin/qmachparser.cpp
+++ b/src/corelib/plugin/qmachparser.cpp
@@ -90,25 +90,24 @@ static int ns(const QString &reason, const QString &library, QString *errorStrin
 
 int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QString *errorString, long *pos, ulong *sectionlen)
 {
-    // we test all possibilities so that we report whether a file is a binary or not
-    const fat_header *fat = reinterpret_cast<const fat_header *>(m_s);
-    const mach_header *mh = reinterpret_cast<const mach_header *>(m_s);
-    const mach_header_64 *mh64 = reinterpret_cast<const mach_header_64 *>(m_s);
-    if (fdlen < sizeof(uint32_t) && fat->magic != qToBigEndian(FAT_MAGIC)
-            && mh->magic != MH_MAGIC && mh->magic != MH_CIGAM
-            && mh64->magic != MH_MAGIC_64 && mh64->magic != MH_CIGAM_64) {
-        if (errorString)
-            *errorString = QLibrary::tr("'%1' is not a Mach-O binary (%2)").arg(library, QLibrary::tr("invalid magic"));
-        return NotSuitable;
-    }
+    // The minimum size of a Mach-O binary we're interested in.
+    // It must have a full Mach header, at least one segment and at least one
+    // section. It's probably useless with just the "qtmetadata" section, but
+    // it's valid nonetheless.
+    // A fat binary must have this plus the fat header, of course.
+    static const size_t MinFileSize = sizeof(my_mach_header) + sizeof(my_segment_command) + sizeof(my_section);
+    static const size_t MinFatHeaderSize = sizeof(fat_header) + 2 * sizeof(fat_arch);
+
+    if (Q_UNLIKELY(fdlen < MinFileSize))
+        return ns(QLibrary::tr("file too small"), library, errorString);
 
     // find out if this is a fat Mach-O binary first
     const my_mach_header *header = 0;
+    const fat_header *fat = reinterpret_cast<const fat_header *>(m_s);
     if (fat->magic == qToBigEndian(FAT_MAGIC)) {
         // find our architecture in the binary
-        const fat_arch *arch = reinterpret_cast<const fat_arch *>(m_s + sizeof(*fat));
-        if (Q_UNLIKELY(fdlen < sizeof(*fat) + 2 * sizeof(*arch))) {
-            // fat binaries must contain at least two architectures
+        const fat_arch *arch = reinterpret_cast<const fat_arch *>(fat + 1);
+        if (Q_UNLIKELY(fdlen < MinFatHeaderSize)) {
             return ns(QLibrary::tr("file too small"), library, errorString);
         }
 
@@ -121,7 +120,8 @@ int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QS
                 // ### should we check the CPU subtype? Maybe on ARM?
                 uint32_t size = qFromBigEndian(arch[i].size);
                 uint32_t offset = qFromBigEndian(arch[i].offset);
-                if (Q_UNLIKELY(size + offset > fdlen || size < sizeof(my_mach_header)))
+                if (Q_UNLIKELY(size > fdlen) || Q_UNLIKELY(offset > fdlen)
+                        || Q_UNLIKELY(size + offset > fdlen) || Q_UNLIKELY(size < MinFileSize))
                     return ns(QString(), library, errorString);
 
                 header = reinterpret_cast<const my_mach_header *>(m_s + offset);
@@ -141,9 +141,8 @@ int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QS
 
         // check magic
         if (header->magic != my_magic)
-            return ns(QLibrary::tr("invalid magic"), library, errorString);
-        if (Q_UNLIKELY(fdlen < sizeof(my_mach_header)))
-            return ns(QString(), library, errorString);
+            return ns(QLibrary::tr("invalid magic %1").arg(qFromBigEndian(header->magic), 8, 16, QLatin1Char('0')),
+                      library, errorString);
     }
 
     // from this point on, fdlen is specific to this architecture
@@ -168,11 +167,16 @@ int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QS
 
     for (uint i = 0; i < header->ncmds; ++i,
          seg = reinterpret_cast<const my_segment_command *>(reinterpret_cast<const char *>(seg) + seg->cmdsize)) {
+        // We're sure that the file size includes at least one load command
+        // but we have to check anyway if we're past the first
         if (Q_UNLIKELY(fdlen < minsize + sizeof(load_command)))
             return ns(QString(), library, errorString);
 
+        // cmdsize can't be trusted until validated
+        // so check it against fdlen anyway
+        // (these are unsigned operations, with overflow behavior specified in the standard)
         minsize += seg->cmdsize;
-        if (Q_UNLIKELY(fdlen < minsize))
+        if (Q_UNLIKELY(fdlen < minsize) || Q_UNLIKELY(fdlen < seg->cmdsize))
             return ns(QString(), library, errorString);
 
         const uint32_t MyLoadCommand = sizeof(void *) > 4 ? LC_SEGMENT_64 : LC_SEGMENT;
@@ -188,7 +192,8 @@ int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QS
                     continue;
 
                 // found it!
-                if (Q_UNLIKELY(fdlen < sect[j].offset + sect[j].size))
+                if (Q_UNLIKELY(fdlen < sect[j].offset) || Q_UNLIKELY(fdlen < sect[j].size)
+                        || Q_UNLIKELY(fdlen < sect[j].offset + sect[j].size))
                     return ns(QString(), library, errorString);
 
                 *pos += sect[j].offset;
diff --git a/tests/auto/corelib/plugin/qpluginloader/machtest/generate-bad.pl b/tests/auto/corelib/plugin/qpluginloader/machtest/generate-bad.pl
new file mode 100755
index 0000000000..ec0dd980a9
--- /dev/null
+++ b/tests/auto/corelib/plugin/qpluginloader/machtest/generate-bad.pl
@@ -0,0 +1,209 @@
+#!/usr/bin/perl
+#############################################################################
+##
+## Copyright (C) 2013 Intel Corporation.
+## Contact: http://www.qt-project.org/legal
+##
+## This file is the build configuration utility of the Qt Toolkit.
+##
+## $QT_BEGIN_LICENSE:LGPL$
+## Commercial License Usage
+## Licensees holding valid commercial Qt licenses may use this file in
+## accordance with the commercial license agreement provided with the
+## Software or, alternatively, in accordance with the terms contained in
+## a written agreement between you and Digia.  For licensing terms and
+## conditions see http://qt.digia.com/licensing.  For further information
+## use the contact form at http://qt.digia.com/contact-us.
+##
+## GNU Lesser General Public License Usage
+## Alternatively, this file may be used under the terms of the GNU Lesser
+## General Public License version 2.1 as published by the Free Software
+## Foundation and appearing in the file LICENSE.LGPL included in the
+## packaging of this file.  Please review the following information to
+## ensure the GNU Lesser General Public License version 2.1 requirements
+## will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+##
+## In addition, as a special exception, Digia gives you certain additional
+## rights.  These rights are described in the Digia Qt LGPL Exception
+## version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+##
+## GNU General Public License Usage
+## Alternatively, this file may be used under the terms of the GNU
+## General Public License version 3.0 as published by the Free Software
+## Foundation and appearing in the file LICENSE.GPL included in the
+## packaging of this file.  Please review the following information to
+## ensure the GNU General Public License version 3.0 requirements will be
+## met: http://www.gnu.org/copyleft/gpl.html.
+##
+##
+## $QT_END_LICENSE$
+##
+#############################################################################
+
+use strict;
+use constant FAT_MAGIC => 0xcafebabe;
+use constant MH_MAGIC => 0xfeedface;
+use constant MH_MAGIC_64 => 0xfeedfacf;
+use constant CPU_TYPE_X86 => 7;
+use constant CPU_TYPE_X86_64 => CPU_TYPE_X86 | 0x01000000;
+use constant CPU_SUBTYPE_I386_ALL => 3;
+use constant MH_DYLIB => 6;
+use constant LC_SEGMENT => 1;
+use constant LC_SEGMENT_64 => 0x19;
+
+my $good = pack("(L7 L2 Z16 L8 Z16 Z16 L9 . L)>",
+    MH_MAGIC, CPU_TYPE_X86, CPU_SUBTYPE_I386_ALL, MH_DYLIB, # 0-3
+    1, # 4: ncmds
+    4 * (37 - 6), # 5: sizeofcmds
+    0, # 6: flags
+
+    LC_SEGMENT, # 7: cmd
+    4 * (37 - 6), # 8: cmdsize
+    '__TEXT', # 9-12: segname
+    0, # 13: vmaddr
+    0x1000, # 14: vmsize
+    0, # 15: fileoff
+    0x204, # 16: filesize
+    7, # 17: maxprot (rwx)
+    5, # 18: initprot (r-x)
+    1, # 19: nsects
+    0, # 20: flags
+
+    'qtmetadata', # 21-24: sectname
+    '__TEXT', # 25-28: segname
+    0x200, # 29: addr
+    4, # 30: size
+    0x200, # 31: offset
+    2, # 32: align (2^2)
+    0, # 33: reloff
+    0, # 34: nreloc
+    0, # 35: flags
+    0, # 36: reserved1
+    0, # 37: reserved2
+
+    0x200,
+    0xc0ffee # data
+);
+
+my $good64 = pack("(L8 L2 Z16 Q4 L4 Z16 Z16 Q2 L8 . Q)>",
+    MH_MAGIC_64, CPU_TYPE_X86_64, CPU_SUBTYPE_I386_ALL, MH_DYLIB, # 0-3
+    1, # 4: ncmds
+    4 * (45 - 7), # 5: sizeofcmds
+    0, # 6: flags
+    0, # 7: reserved
+
+    LC_SEGMENT_64, # 8: cmd
+    4 * (45 - 7), # 9: cmdsize
+    '__TEXT', # 10-13: segname
+    0, # 14-15: vmaddr
+    0x1000, # 16-17: vmsize
+    0, # 18-19: fileoff
+    0x208, # 20-21: filesize
+    7, # 22: maxprot (rwx)
+    5, # 23: initprot (r-x)
+    1, # 24: nsects
+    0, # 25: flags
+
+    'qtmetadata', # 26-29: sectname
+    '__TEXT', # 30-33: segname
+    0x200, # 34-35: addr
+    4, # 36-37: size
+    0x200, # 38: offset
+    3, # 39: align (2^3)
+    0, # 40: reloff
+    0, # 41: nreloc
+    0, # 42: flags
+    0, # 43: reserved1
+    0, # 44: reserved2
+    0, # 45: reserved3
+
+    0x200,
+    0xc0ffeec0ffee # data
+);
+
+my $fat = pack("L>*",
+    FAT_MAGIC, # 1: magic
+    2, # 2: nfat_arch
+
+    CPU_TYPE_X86, # 3: cputype
+    CPU_SUBTYPE_I386_ALL, # 4: cpusubtype
+    0x1000, # 5: offset
+    0x1000, # 6: size
+    12, # 7: align (2^12)
+
+    CPU_TYPE_X86_64, # 8: cputype
+    CPU_SUBTYPE_I386_ALL, # 9: cpusubtype
+    0x2000, # 10: offset
+    0x1000, # 11: size
+    12, # 12: align (2^12)
+);
+
+my $buffer;
+
+our $badcount = 1;
+sub generate($) {
+    open OUT, ">", "bad$badcount.dylib" or die("Could not open file bad$badcount.dylib: $!\n");
+    binmode OUT;
+    print OUT $_[0];
+    close OUT;
+    ++$badcount;
+}
+
+# Bad file 1-2
+# Except that the cmdsize fields are null
+$buffer = $good;
+vec($buffer, 5, 32) = 0;
+generate $buffer;
+
+$buffer = $good;
+vec($buffer, 8, 32) = 0;
+generate $buffer;
+
+# Bad file 3-4: same as above but 64-bit
+$buffer = $good64;
+vec($buffer, 5, 32) = 0;
+generate $buffer;
+
+$buffer = $good64;
+vec($buffer, 9, 32) = 0;
+generate $buffer;
+
+# Bad file 5-8: same as 1-4, but set cmdsize to bigger than file
+$buffer = $good;
+vec($buffer, 5, 32) = 0x1000;
+generate $buffer;
+
+$buffer = $good;
+vec($buffer, 8, 32) = 0x1000;
+generate $buffer;
+
+$buffer = $good64;
+vec($buffer, 5, 32) = 0x1000;
+generate $buffer;
+
+$buffer = $good64;
+vec($buffer, 9, 32) = 0x1000;
+generate $buffer;
+
+# Bad file 9-10: overflow size+offset
+$buffer = $good;
+vec($buffer, 30, 32) = 0xffffffe0;
+generate $buffer;
+
+$buffer = $good64;
+vec($buffer, 36, 32) = 0xffffffff;
+vec($buffer, 37, 32) = 0xffffffe0;
+generate $buffer;
+
+# Bad file 11: FAT binary with just the header
+generate $fat;
+
+# Bad file 12: FAT binary where the Mach contents don't match the FAT directory
+$buffer = pack("a4096 a4096 a4096", $fat, $good64, $good);
+generate $buffer;
+
+# Bad file 13: FAT binary with overflowing size
+$buffer = pack("a4096 a4096 a4096", $fat, $good, $good64);
+vec($buffer, 5, 32) = 0xfffffffe0;
+vec($buffer, 10, 32) = 0xfffffffe0;
+generate $buffer;
diff --git a/tests/auto/corelib/plugin/qpluginloader/machtest/machtest.pro b/tests/auto/corelib/plugin/qpluginloader/machtest/machtest.pro
index d4fc9f2836..b1183da84d 100644
--- a/tests/auto/corelib/plugin/qpluginloader/machtest/machtest.pro
+++ b/tests/auto/corelib/plugin/qpluginloader/machtest/machtest.pro
@@ -1,6 +1,7 @@
 TEMPLATE = aux
 OTHER_FILES += \
-    ppcconverter.pl
+    ppcconverter.pl \
+    generate-bad.pl
 
 i386.target = good.i386.dylib
 i386.commands = $(CXX) -shared -arch i386 -o $@ -I$$[QT_INSTALL_HEADERS/get] $<
@@ -39,13 +40,17 @@ fat_stub_x86_64.target = good.fat.stub-x86_64.dylib
 fat_stub_x86_64.commands = lipo -create -output $@ -arch ppc64 $$ppc64.target -arch_blank x86_64
 fat_stub_x86_64.depends += i386 ppc64
 
+bad.commands = $$PWD/generate-bad.pl
+bad.depends += $$PWD/generate-bad.pl
+
 MYTARGETS = $$fat_all.depends fat_all fat_no_x86_64 fat_no_i386 \
-            fat_stub_i386 fat_stub_x86_64
+            fat_stub_i386 fat_stub_x86_64 bad
 all.depends += $$MYTARGETS
 QMAKE_EXTRA_TARGETS += $$MYTARGETS all
 
 QMAKE_CLEAN += $$i386.target $$x86_64.target $$ppc64.target $$fat_all.target \
             $$fat_no_i386.target $$fat_no_x86_64.target \
-            $$fat_stub_i386.target $$fat_stub_x86_64.target
+            $$fat_stub_i386.target $$fat_stub_x86_64.target \
+            "bad*.dylib"
 
 
diff --git a/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp b/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp
index 989d311ebf..351e3a23e0 100644
--- a/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp
+++ b/tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp
@@ -345,6 +345,11 @@ void tst_QPluginLoader::loadMachO_data()
     QTest::newRow("machtest/good.fat.all.dylib") << int(QMachOParser::QtMetaDataSection);
     QTest::newRow("machtest/good.fat.stub-x86_64.dylib") << int(QMachOParser::NotSuitable);
     QTest::newRow("machtest/good.fat.stub-i386.dylib") << int(QMachOParser::NotSuitable);
+
+    QDir d(QFINDTESTDATA("machtest"));
+    QStringList badlist = d.entryList(QStringList() << "bad*.dylib");
+    foreach (const QString &bad, badlist)
+        QTest::newRow(qPrintable("machtest/" + bad)) << int(QMachOParser::NotSuitable);
 #endif
 }