From 2f333424a67084fab02e7550930ca55b36911566 Mon Sep 17 00:00:00 2001 From: Eladash Date: Sun, 21 Mar 2021 07:03:21 +0200 Subject: [PATCH] Improve ELF/Trophy loader's error checking --- Utilities/File.cpp | 11 +++++++++++ Utilities/File.h | 35 +++++++++++++++++++++++++++-------- rpcs3/Loader/ELF.h | 9 +++------ rpcs3/Loader/PUP.cpp | 14 +++----------- rpcs3/Loader/TROPUSR.cpp | 22 ++++++---------------- rpcs3/Loader/TRP.cpp | 23 +++++++++++------------ 6 files changed, 61 insertions(+), 53 deletions(-) diff --git a/Utilities/File.cpp b/Utilities/File.cpp index 7793d22411..a790bc5308 100644 --- a/Utilities/File.cpp +++ b/Utilities/File.cpp @@ -1556,6 +1556,17 @@ bool fs::dir::open(const std::string& path) return true; } +bool fs::file::strict_read_check(u64 _size, u64 type_size) const +{ + if (usz pos0 = pos(), size0 = size(); pos0 >= size0 || (size0 - pos0) / type_size < _size) + { + fs::g_tls_error = fs::error::inval; + return false; + } + + return true; +} + const std::string& fs::get_config_dir() { // Use magic static diff --git a/Utilities/File.h b/Utilities/File.h index 6206e2abdc..f17671c9d0 100644 --- a/Utilities/File.h +++ b/Utilities/File.h @@ -200,6 +200,8 @@ namespace fs { std::unique_ptr m_file; + bool strict_read_check(u64 size, u64 type_size) const; + public: // Default constructor file() = default; @@ -377,15 +379,24 @@ namespace fs } // Read std::basic_string - template - std::enable_if_t && !std::is_pointer_v, bool> read(std::basic_string& str, usz size, + template + std::enable_if_t && !std::is_pointer_v, bool> read(std::basic_string& str, usz _size, const char* file = __builtin_FILE(), const char* func = __builtin_FUNCTION(), u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN()) const { - str.resize(size); - return read(&str[0], size * sizeof(T), line, col, file, func) == size * sizeof(T); + if (!m_file) xnull({line, col, file, func}); + if (!_size) return true; + + if constexpr (IsStrict) + { + // If _size arg is too high std::bad_alloc may happen in resize and then we cannot error check + if (!strict_read_check(_size, sizeof(T))) return false; + } + + str.resize(_size); + return read(str.data(), sizeof(T) * _size, line, col, file, func) == sizeof(T) * _size; } // Read POD, sizeof(T) is used @@ -412,15 +423,23 @@ namespace fs } // Read POD std::vector - template - std::enable_if_t && !std::is_pointer_v, bool> read(std::vector& vec, usz size, + template + std::enable_if_t && !std::is_pointer_v, bool> read(std::vector& vec, usz _size, const char* file = __builtin_FILE(), const char* func = __builtin_FUNCTION(), u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN()) const { - vec.resize(size); - return read(vec.data(), sizeof(T) * size, line, col, file, func) == sizeof(T) * size; + if (!m_file) xnull({line, col, file, func}); + if (!_size) return true; + + if constexpr (IsStrict) + { + if (!strict_read_check(_size, sizeof(T))) return false; + } + + vec.resize(_size); + return read(vec.data(), sizeof(T) * _size, line, col, file, func) == sizeof(T) * _size; } // Read POD (experimental) diff --git a/rpcs3/Loader/ELF.h b/rpcs3/Loader/ELF.h index 8757ce1b75..daf67f3ddf 100644 --- a/rpcs3/Loader/ELF.h +++ b/rpcs3/Loader/ELF.h @@ -241,17 +241,15 @@ public: if (!(opts & elf_opt::no_programs)) { - _phdrs.resize(header.e_phnum); stream.seek(offset + header.e_phoff); - if (!stream.read(_phdrs)) + if (!stream.read(_phdrs, header.e_phnum)) return set_error(elf_error::stream_phdrs); } if (!(opts & elf_opt::no_sections)) { - shdrs.resize(header.e_shnum); stream.seek(offset + header.e_shoff); - if (!stream.read(shdrs)) + if (!stream.read(shdrs, header.e_shnum)) return set_error(elf_error::stream_shdrs); } @@ -265,9 +263,8 @@ public: if (!(opts & elf_opt::no_data)) { - progs.back().bin.resize(hdr.p_filesz); stream.seek(offset + hdr.p_offset); - if (!stream.read(progs.back().bin)) + if (!stream.read(progs.back().bin, hdr.p_filesz)) return set_error(elf_error::stream_data); } } diff --git a/rpcs3/Loader/PUP.cpp b/rpcs3/Loader/PUP.cpp index 7066bf1c77..65af45ef01 100644 --- a/rpcs3/Loader/PUP.cpp +++ b/rpcs3/Loader/PUP.cpp @@ -40,23 +40,15 @@ pup_object::pup_object(fs::file&& file) : m_file(std::move(file)) return; } - constexpr usz entry_size = sizeof(PUPFileEntry) + sizeof(PUPHashEntry); - - if (!m_header.file_count || (file_size - sizeof(PUPHeader)) / entry_size < m_header.file_count) + if (!m_header.file_count) { - // These checks before read() are to avoid some std::bad_alloc exceptions when file_count is too large - // So we cannot rely on read() for error checking in such cases m_error = pup_error::header_file_count; return; } - m_file_tbl.resize(m_header.file_count); - m_hash_tbl.resize(m_header.file_count); - - if (!m_file.read(m_file_tbl) || !m_file.read(m_hash_tbl)) + if (!m_file.read(m_file_tbl, m_header.file_count) || !m_file.read(m_hash_tbl, m_header.file_count)) { - // If these fail it is an unexpected filesystem error, because file size must suffice as we checked in previous checks - m_error = pup_error::file_entries; + m_error = pup_error::header_file_count; return; } diff --git a/rpcs3/Loader/TROPUSR.cpp b/rpcs3/Loader/TROPUSR.cpp index 1eb0c7955a..046ada451a 100644 --- a/rpcs3/Loader/TROPUSR.cpp +++ b/rpcs3/Loader/TROPUSR.cpp @@ -52,12 +52,10 @@ bool TROPUSRLoader::LoadTableHeaders() m_file.seek(0x30); m_tableHeaders.clear(); - m_tableHeaders.resize(m_header.tables_count); - for (TROPUSRTableHeader& tableHeader : m_tableHeaders) + if (!m_file.read(m_tableHeaders, m_header.tables_count)) { - if (!m_file.read(tableHeader)) - return false; + return false; } return true; @@ -77,25 +75,17 @@ bool TROPUSRLoader::LoadTables() if (tableHeader.type == 4u) { m_table4.clear(); - m_table4.resize(tableHeader.entries_count); - for (auto& entry : m_table4) - { - if (!m_file.read(entry)) - return false; - } + if (!m_file.read(m_table4, tableHeader.entries_count)) + return false; } if (tableHeader.type == 6u) { m_table6.clear(); - m_table6.resize(tableHeader.entries_count); - for (auto& entry : m_table6) - { - if (!m_file.read(entry)) - return false; - } + if (!m_file.read(m_table6, tableHeader.entries_count)) + return false; } // TODO: Other tables diff --git a/rpcs3/Loader/TRP.cpp b/rpcs3/Loader/TRP.cpp index 655eaf651a..29f80241d4 100644 --- a/rpcs3/Loader/TRP.cpp +++ b/rpcs3/Loader/TRP.cpp @@ -39,8 +39,8 @@ bool TRPLoader::Install(const std::string& dest, bool /*show*/) for (const TRPEntry& entry : m_entries) { trp_f.seek(entry.offset); - buffer.resize(entry.size); - if (!trp_f.read(buffer)) + + if (!trp_f.read(buffer, entry.size)) { trp_log.error("Failed to read TRPEntry at: offset=0x%x, size=0x%x", entry.offset, entry.size); continue; // ??? @@ -103,10 +103,10 @@ bool TRPLoader::LoadHeader(bool show) if (m_header.trp_version >= 2) { unsigned char hash[20]; - std::vector file_contents(m_header.trp_file_size); + std::vector file_contents; trp_f.seek(0); - if (!trp_f.read(file_contents)) + if (!trp_f.read(file_contents, m_header.trp_file_size)) { trp_log.notice("Failed verifying checksum"); } @@ -126,18 +126,17 @@ bool TRPLoader::LoadHeader(bool show) } m_entries.clear(); - m_entries.resize(m_header.trp_files_count); - for (u32 i = 0; i < m_header.trp_files_count; i++) + if (!trp_f.read(m_entries, m_header.trp_files_count)) { - if (!trp_f.read(m_entries[i])) - { - return false; - } + return false; + } - if (show) + if (show) + { + for (const auto& entry : m_entries) { - trp_log.notice("TRP entry #%d: %s", m_entries[i].name); + trp_log.notice("TRP entry #%u: %s", &entry - m_entries.data(), entry.name); } }