From 2e2cf2e495ae496a2e79881cf5797e17ca7544d6 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Wed, 15 Feb 2023 15:45:10 -0800 Subject: [PATCH] Fix ICU post build checks. (#905) try_read_image_config_directory checked for a LOAD_CONFIG data directory, but didn't check that it was non-nullptr, like the import and export checks did. Extract a common function to make it easier to verify that we always do this. C:\Dev>dumpbin /imports "C:\Dev\vcpkg\packages\icu_x64-windows\bin\icudt72.dll" Microsoft (R) COFF/PE Dumper Version 14.34.31942.0 Copyright (C) Microsoft Corporation. All rights reserved. Dump of file C:\Dev\vcpkg\packages\icu_x64-windows\bin\icudt72.dll File Type: DLL Summary 1DCE000 .rdata 1000 .rsrc C:\Dev> --- include/vcpkg/base/cofffilereader.h | 1 + src/vcpkg/base/cofffilereader.cpp | 54 +++++++++++++++-------------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/include/vcpkg/base/cofffilereader.h b/include/vcpkg/base/cofffilereader.h index 63af58278c..d433e1e26c 100644 --- a/include/vcpkg/base/cofffilereader.h +++ b/include/vcpkg/base/cofffilereader.h @@ -361,6 +361,7 @@ namespace vcpkg std::vector data_directories; std::vector section_headers; + const ImageDataDirectory* try_get_image_data_directory(size_t directory_index) const noexcept; MachineType get_machine_type() const noexcept; bool is_arm64_ec() const noexcept; }; diff --git a/src/vcpkg/base/cofffilereader.cpp b/src/vcpkg/base/cofffilereader.cpp index 0234dd7d09..1a1faea17f 100644 --- a/src/vcpkg/base/cofffilereader.cpp +++ b/src/vcpkg/base/cofffilereader.cpp @@ -138,18 +138,17 @@ namespace ExpectedL try_read_image_config_directory(DllMetadata& metadata, ReadFilePointer& f) { - // pre: try_read_section_headers succeeded on `metadata` - if (metadata.data_directories.size() < 11) + const auto load_config_data_directory = metadata.try_get_image_data_directory(10); + if (!load_config_data_directory) { // metadata.load_config_type = LoadConfigType::UnsetOrOld; return Unit{}; } - const auto& load_config_data_directory = metadata.data_directories[10]; - auto maybe_remaining_size = try_seek_to_rva(metadata, f, load_config_data_directory.virtual_address); + auto maybe_remaining_size = try_seek_to_rva(metadata, f, load_config_data_directory->virtual_address); if (const auto remaining_size = maybe_remaining_size.get()) { - if (*remaining_size < load_config_data_directory.size) + if (*remaining_size < load_config_data_directory->size) { return msg::format(msgPEConfigCrossesSectionBoundary, msg::path = f.path()); } @@ -529,6 +528,21 @@ namespace vcpkg } } + const ImageDataDirectory* DllMetadata::try_get_image_data_directory(size_t directory_index) const noexcept + { + // pre: try_read_section_headers succeeded on `metadata` + if (data_directories.size() > directory_index) + { + const auto result = data_directories.data() + directory_index; + if (result->virtual_address != 0) + { + return result; + } + } + + return nullptr; + } + MachineType DllMetadata::get_machine_type() const noexcept { return static_cast(coff_header.machine); } uint64_t ArchiveMemberHeader::decoded_size() const @@ -675,22 +689,16 @@ namespace vcpkg ExpectedL try_read_if_dll_has_exports(const DllMetadata& dll, ReadFilePointer& f) { - if (dll.data_directories.size() < 1) - { - Debug::print("No export directory\n"); - return false; - } - - const auto& export_data_directory = dll.data_directories[0]; - if (export_data_directory.virtual_address == 0) + const auto export_data_directory = dll.try_get_image_data_directory(0); + if (!export_data_directory) { - Debug::print("Null export directory.\n"); + Debug::print("No export directory.\n"); return false; } ExportDirectoryTable export_directory_table; auto export_read_result = try_read_struct_from_rva( - dll, f, &export_directory_table, export_data_directory.virtual_address, sizeof(export_directory_table)); + dll, f, &export_directory_table, export_data_directory->virtual_address, sizeof(export_directory_table)); if (!export_read_result.has_value()) { return std::move(export_read_result).error(); @@ -701,28 +709,22 @@ namespace vcpkg ExpectedL> try_read_dll_imported_dll_names(const DllMetadata& dll, ReadFilePointer& f) { - if (dll.data_directories.size() < 2) + const auto import_data_directory = dll.try_get_image_data_directory(1); + if (!import_data_directory) { Debug::print("No import directory\n"); return std::vector{}; } - const auto& import_data_directory = dll.data_directories[1]; - if (import_data_directory.virtual_address == 0) - { - Debug::print("Null import directory\n"); - return std::vector{}; - } - - return try_seek_to_rva(dll, f, import_data_directory.virtual_address) + return try_seek_to_rva(dll, f, import_data_directory->virtual_address) .then([&](size_t remaining_size) -> ExpectedL> { - if (remaining_size < import_data_directory.size) + if (remaining_size < import_data_directory->size) { return msg::format(msgPEImportCrossesSectionBoundary, msg::path = f.path()); } std::vector results; - remaining_size = import_data_directory.size; + remaining_size = import_data_directory->size; if (remaining_size < sizeof(ImportDirectoryTableEntry)) { Debug::print("No import directory table entries");