hardening & refactoring

My latest obsession is integer overflows.

4 files changed, 99 insertions, 66 deletions

diff --git a/include/pdb/module.hpp b/include/pdb/module.hpp
index 3e785c4..e539fde 100644
--- a/include/pdb/module.hpp
+++ b/include/pdb/module.hpp
@@ -10,10 +10,6 @@
 #include <Windows.h>
 #include <DbgHelp.h>
 
-#include <cstring>
-
-#include <sstream>
-#include <stdexcept>
 #include <string>
 
 namespace pdb
@@ -23,16 +19,10 @@ namespace pdb
     public:
         typedef IMAGEHLP_MODULE64 Raw;
 
-        ModuleInfo()
-            : raw{prepare_buffer()}
-        { }
-
-        explicit ModuleInfo(const Raw& raw)
-            : raw{raw}
-        { }
+        ModuleInfo();
+        explicit ModuleInfo(const Raw& raw);
 
         explicit operator Raw&() { return raw; }
-
         explicit operator const Raw&() const { return raw; }
 
         Address get_offline_base() const { return raw.BaseOfImage; }
@@ -40,13 +30,7 @@ namespace pdb
         std::string get_name() const { return raw.ModuleName; }
 
     private:
-        static Raw prepare_buffer()
-        {
-            Raw raw;
-            std::memset(&raw, 0, sizeof(raw));
-            raw.SizeOfStruct = sizeof(raw);
-            return raw;
-        }
+        static Raw create_raw();
 
         Raw raw;
     };
@@ -61,38 +45,12 @@ namespace pdb
 
         Address get_online_base() const { return online_base; }
 
-        Address translate_offline_address(Address offline) const
-        {
-            if (offline < get_offline_base())
-                throw std::range_error{invalid_offline_address(offline)};
-            return offline - get_offline_base() + get_online_base();
-        }
-
-        Address translate_online_address(Address online) const
-        {
-            if (online < get_online_base())
-                throw std::range_error{invalid_online_address(online)};
-            return online - get_online_base() + get_offline_base();
-        }
+        Address translate_offline_address(Address offline) const;
+        Address translate_online_address(Address online) const;
 
     private:
-        std::string invalid_offline_address(Address offline) const
-        {
-            std::ostringstream oss;
-            oss << "offline address " << format_address(offline)
-                << " doesn't belong to module " << get_name()
-                << " (base offline address " << format_address(get_offline_base()) << ')';
-            return oss.str();
-        }
-
-        std::string invalid_online_address(Address online) const
-        {
-            std::ostringstream oss;
-            oss << "online address " << format_address(online)
-                << " doesn't belong to module " << get_name()
-                << " (base online address " << format_address(get_online_base()) << ')';
-            return oss.str();
-        }
+        std::string invalid_offline_address(Address offline) const;
+        std::string invalid_online_address(Address online) const;
 
         const Address online_base;
     };
diff --git a/src/dbghelp.cpp b/src/dbghelp.cpp
index ff8618f..cd9b765 100644
--- a/src/dbghelp.cpp
+++ b/src/dbghelp.cpp
@@ -5,6 +5,8 @@
 
 #include "pdb/all.hpp"
 
+#include <safeint.h>
+
 #include <Windows.h>
 #include <DbgHelp.h>
 
@@ -41,9 +43,10 @@ namespace pdb
         Address gen_next_offline_base(std::size_t pdb_size)
         {
             static Address id = 0x10000000;
-            const auto next = id;
-            id += pdb_size;
-            return next;
+            const auto base = id;
+            if (!msl::utilities::SafeAdd(id, pdb_size, id))
+                throw std::runtime_error{"no more PDB files can be added, the internal address space is exhausted"};
+            return base;
         }
 
         BOOL CALLBACK enum_symbols_callback(
@@ -65,10 +68,9 @@ namespace pdb
 
     ModuleInfo DbgHelp::load_pdb(const std::string& path) const
     {
-        const auto size = file::get_size(path);
-
-        if (size > std::numeric_limits<DWORD>::max())
-            throw std::range_error{"PDB file size is too large"};
+        DWORD size = 0;
+        if (!msl::utilities::SafeCast(file::get_size(path), size))
+            throw std::range_error{"PDB file is too large"};
 
         const auto offline_base = SymLoadModule64(
             id,
@@ -76,7 +78,7 @@ namespace pdb
             path.c_str(),
             NULL,
             gen_next_offline_base(size),
-            static_cast<DWORD>(size));
+            size);
 
         if (!offline_base)
             throw error::windows(GetLastError());
diff --git a/src/module.cpp b/src/module.cpp
new file mode 100644
index 0000000..3694e5f
--- /dev/null
+++ b/src/module.cpp
@@ -0,0 +1,76 @@
+// Copyright (c) 2017 Egor Tensin <Egor.Tensin@gmail.com>
+// This file is part of the "PDB repository" project.
+// For details, see https://github.com/egor-tensin/pdb-repo.
+// Distributed under the MIT License.
+
+#include "pdb/all.hpp"
+
+#include <safeint.h>
+
+#include <cstring>
+
+#include <sstream>
+#include <stdexcept>
+#include <string>
+
+namespace pdb
+{
+    ModuleInfo::ModuleInfo()
+        : ModuleInfo{create_raw()}
+    { }
+
+    ModuleInfo::ModuleInfo(const Raw& raw)
+        : raw{raw}
+    {
+        if (raw.SizeOfStruct != sizeof(raw))
+            throw std::runtime_error{"unexpected module structure size"};
+    }
+
+    ModuleInfo::Raw ModuleInfo::create_raw()
+    {
+        Raw raw;
+        std::memset(&raw, 0, sizeof(raw));
+        raw.SizeOfStruct = sizeof(raw);
+        return raw;
+    }
+
+    Address Module::translate_offline_address(Address offline) const
+    {
+        if (offline < get_offline_base())
+            throw std::range_error{invalid_offline_address(offline)};
+        const auto offset = offline - get_offline_base();
+        auto online = offset;
+        if (!msl::utilities::SafeAdd(online, get_online_base(), online))
+            throw std::range_error{invalid_offline_address(offline)};
+        return online;
+    }
+
+    Address Module::translate_online_address(Address online) const
+    {
+        if (online < get_online_base())
+            throw std::range_error{invalid_online_address(online)};
+        const auto offset = online - get_online_base();
+        auto offline = offset;
+        if (!msl::utilities::SafeAdd(offline, get_offline_base(), offline))
+            throw std::range_error{invalid_online_address(offline)};
+        return offline;
+    }
+
+    std::string Module::invalid_offline_address(Address offline) const
+    {
+        std::ostringstream oss;
+        oss << "offline address " << format_address(offline)
+            << " doesn't belong to module " << get_name()
+            << " (base offline address " << format_address(get_offline_base()) << ')';
+        return oss.str();
+    }
+
+    std::string Module::invalid_online_address(Address online) const
+    {
+        std::ostringstream oss;
+        oss << "online address " << format_address(online)
+            << " doesn't belong to module " << get_name()
+            << " (base online address " << format_address(get_online_base()) << ')';
+        return oss.str();
+    }
+}
diff --git a/src/utils/file.cpp b/src/utils/file.cpp
index 4150685..f013d90 100644
--- a/src/utils/file.cpp
+++ b/src/utils/file.cpp
@@ -37,15 +37,12 @@ namespace pdb
             if (!GetFileSizeEx(handle.get(), &size))
                 throw error::windows(GetLastError());
 
-            try
-            {
-                const msl::utilities::SafeInt<decltype(size.QuadPart)> safe_size{size.QuadPart};
-                return static_cast<std::size_t>(safe_size);
-            }
-            catch (const msl::utilities::SafeIntException&)
-            {
-                throw std::range_error{"invalid file size"};
-            }
+            std::size_t result = 0;
+
+            if (!msl::utilities::SafeCast(size.QuadPart, result))
+                throw std::runtime_error{"unsupported file size"};
+
+            return result;
         }
     }
 }