From d48f6281af49b17d1f6d6802b8921c1aed38a713 Mon Sep 17 00:00:00 2001 From: Egor Tensin Date: Fri, 19 May 2017 06:48:22 +0300 Subject: hardening & refactoring My latest obsession is integer overflows. --- include/pdb/module.hpp | 56 +++++-------------------------------- src/dbghelp.cpp | 18 ++++++------ src/module.cpp | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/utils/file.cpp | 15 ++++------ 4 files changed, 99 insertions(+), 66 deletions(-) create mode 100644 src/module.cpp 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 #include -#include - -#include -#include #include 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 + #include #include @@ -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::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(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 +// 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 + +#include + +#include +#include +#include + +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 safe_size{size.QuadPart}; - return static_cast(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; } } } -- cgit v1.2.3