Improve firmware installation error handling

* Add new error types and descriptions.
* Do not crash on missing 0x100 and 0x300 PUP file entries.
* Report an error on missing PUP package inner files.
* Fix overflow in file-size against header check.
* Move all header errors to pup_object class.
* Move verbose error descriptions to pup_object class.
* Minor optimizations.

rpcs3/Loader/PUP.cpp

@@ -5,28 +5,48 @@
 
 #include "PUP.h"
 
-pup_object::pup_object(const fs::file& file): m_file(file)
+pup_object::pup_object(fs::file&& file) : m_file(std::move(file))
 {
-	if (!file)
+	if (!m_file)
 	{
+		m_error = pup_error::stream;
 		return;
 	}
 
 	m_file.seek(0);
 	PUPHeader m_header{};
 
-	if (!m_file.read(m_header) || m_header.magic != "SCEUF\0\0\0"_u64)
+	const usz file_size = m_file.size();
+
+	if (!m_file.read(m_header))
 	{
-		// Either file is not large enough to contain header or magic is invalid
+		// File is not large enough to contain header or magic is invalid
+		m_error = pup_error::header_read;
+		m_formatted_error = fmt::format("Too small PUP file to contain header: 0x%x", file_size);
+		return;
+	}
+
+	if (m_header.magic != "SCEUF\0\0\0"_u64)
+	{
+		m_error = pup_error::header_magic;
+		return;
+	}
+
+	// Check if file size is the expected size, use substraction to avoid overflows
+	if (file_size < m_header.header_length || file_size - m_header.header_length < m_header.data_length)
+	{
+		m_formatted_error = fmt::format("Firmware size mismatch, expected: 0x%x + 0x%x, actual: 0x%x", m_header.header_length, m_header.data_length, file_size);
+		m_error = pup_error::expected_size;
 		return;
 	}
 
 	constexpr usz entry_size = sizeof(PUPFileEntry) + sizeof(PUPHashEntry);
 
-	if (!m_header.file_count || (m_file.size() - sizeof(PUPHeader)) / entry_size < m_header.file_count)
+	if (!m_header.file_count || (file_size - sizeof(PUPHeader)) / entry_size < 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;
 	}
 
@@ -36,17 +56,24 @@ pup_object::pup_object(const fs::file& file): m_file(file)
 	if (!m_file.read(m_file_tbl) || !m_file.read(m_hash_tbl))
 	{
 		// 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;
 		return;
 	}
 
-	isValid = true;
+	if (pup_error err = validate_hashes(); err != pup_error::ok)
+	{
+		m_error = err;
+		return;
+	}
+
+	m_error = pup_error::ok;
 }
 
-fs::file pup_object::get_file(u64 entry_id)
+fs::file pup_object::get_file(u64 entry_id) const
 {
-	if (!isValid) return fs::file();
+	if (m_error != pup_error::ok) return {};
 
-	for (PUPFileEntry file_entry : m_file_tbl)
+	for (const PUPFileEntry& file_entry : m_file_tbl)
 	{
 		if (file_entry.entry_id == entry_id)
 		{
@@ -56,34 +83,41 @@ fs::file pup_object::get_file(u64 entry_id)
 			return fs::make_stream(std::move(file_buf));
 		}
 	}
-	return fs::file();
+
+	return {};
 }
 
-bool pup_object::validate_hashes()
+pup_error pup_object::validate_hashes()
 {
-	if (!isValid) return false;
+	AUDIT(m_error == pup_error::ok);
 
-	for (usz i = 0; i < m_file_tbl.size(); i++)
+	std::vector<u8> buffer;
+
+	const usz size = m_file.size();
+
+	for (const PUPFileEntry& file : m_file_tbl)
 	{
-		u8 *hash = m_hash_tbl[i].hash;
-		PUPFileEntry file = m_file_tbl[i];
-
 		// Sanity check for offset and length, use substraction to avoid overflows
-		if (usz size = m_file.size(); size < file.data_offset || size - file.data_offset < file.data_length)
+		if (size < file.data_offset || size - file.data_offset < file.data_length)
 		{
-			return false;
+			m_formatted_error = fmt::format("File database entry is invalid. (offset=0x%x, length=0x%x, PUP.size=0x%x)", file.data_offset, file.data_length, size);
+			return pup_error::file_entries;
 		}
 
-		std::vector<u8> buffer(file.data_length);
+		// Reuse buffer
+		buffer.resize(file.data_length);
 		m_file.seek(file.data_offset);
 		m_file.read(buffer.data(), file.data_length);
 
 		u8 output[20] = {};
 		sha1_hmac(PUP_KEY, sizeof(PUP_KEY), buffer.data(), buffer.size(), output);
-		if (memcmp(output, hash, 20) != 0)
+
+		// Compare to hash entry
+		if (std::memcmp(output, m_hash_tbl[&file - m_file_tbl.data()].hash, 20) != 0)
 		{
-			return false;
+			return pup_error::hash_mismatch;
 		}
 	}
-	return true;
+
+	return pup_error::ok;
 }

rpcs3/Loader/PUP.h

@@ -30,19 +30,37 @@ struct PUPHashEntry
 	u8 padding[4];
 };
 
+// PUP loading error
+enum class pup_error : u32
+{
+	ok,
+
+	stream,
+	header_read,
+	header_magic,
+	header_file_count,
+	expected_size,
+	file_entries,
+	hash_mismatch,
+};
+
 class pup_object
 {
-	const fs::file& m_file;
+	fs::file m_file;
-	bool isValid = false;
+
+	pup_error m_error{};
+	std::string m_formatted_error;
 
 	std::vector<PUPFileEntry> m_file_tbl;
 	std::vector<PUPHashEntry> m_hash_tbl;
 
+	pup_error validate_hashes();
+
 public:
-	pup_object(const fs::file& file);
+	pup_object(fs::file&& file);
 
-	explicit operator bool() const { return isValid; }
+	explicit operator pup_error() const { return m_error; }
+	const std::string& get_formatted_error() const { return m_formatted_error; }
 
-	fs::file get_file(u64 entry_id);
+	fs::file get_file(u64 entry_id) const;
-	bool validate_hashes();
 };

rpcs3/rpcs3qt/main_window.cpp

@@ -865,45 +865,66 @@ void main_window::HandlePupInstallation(QString file_path)
 	fs::file pup_f(path);
 	if (!pup_f)
 	{
-		gui_log.error("Error opening PUP file %s", path);
+		gui_log.error("Error opening PUP file %s (%s)", path, fs::g_tls_error);
 		critical(tr("Firmware installation failed: The selected firmware file couldn't be opened."));
 		return;
 	}
 
-	if (pup_f.size() < sizeof(PUPHeader))
+	pup_object pup(std::move(pup_f));
+
+	switch (pup.operator pup_error())
 	{
-		gui_log.error("Too small PUP file: %llu", pup_f.size());
+	case pup_error::header_read:
+	{
+		gui_log.error("%s", pup.get_formatted_error());
 		critical(tr("Firmware installation failed: The provided file is empty."));
 		return;
 	}
-
+	case pup_error::header_magic:
-	struct PUPHeader header = {};
-	pup_f.seek(0);
-	pup_f.read(header);
-
-	if (header.header_length + header.data_length != pup_f.size())
 	{
-		gui_log.error("Firmware size mismatch, expected: %llu + %llu, actual: %llu", header.header_length, header.data_length, pup_f.size());
+		gui_log.error("Error while installing firmware: provided file is not a PUP file.");
+		critical(tr("Firmware installation failed: The provided file is not a PUP file."));
+		return;
+	}
+	case pup_error::expected_size:
+	{
+		gui_log.error("%s", pup.get_formatted_error());
 		critical(tr("Firmware installation failed: The provided file is incomplete. Try redownloading it."));
 		return;
 	}
-
+	case pup_error::header_file_count:
-	pup_object pup(pup_f);
+	case pup_error::file_entries:
-	if (!pup)
+	default:
 	{
-		gui_log.error("Error while installing firmware: PUP file is invalid.");
+		std::string error = "Error while installing firmware: PUP file is invalid.";
+
+		if (!pup.get_formatted_error().empty())
+		{
+			fmt::append(error, "\n%s", pup.get_formatted_error());
+		}
+
+		gui_log.error("%s", error);
 		critical(tr("Firmware installation failed: The provided file is corrupted."));
 		return;
 	}
-
+	case pup_error::hash_mismatch:
-	if (!pup.validate_hashes())
 	{
 		gui_log.error("Error while installing firmware: Hash check failed.");
 		critical(tr("Firmware installation failed: The provided file's contents are corrupted."));
 		return;
 	}
+	case pup_error::ok: break;
+	}
 
 	fs::file update_files_f = pup.get_file(0x300);
+
+	if (!update_files_f)
+	{
+		gui_log.error("Error while installing firmware: Couldn't find installation packages database.");
+		critical(tr("Firmware installation failed: The provided file's contents are corrupted."));
+		return;
+	}
+
 	tar_object update_files(update_files_f);
 	auto update_filenames = update_files.get_filenames();
 
@@ -911,17 +932,36 @@ void main_window::HandlePupInstallation(QString file_path)
 		update_filenames.begin(), update_filenames.end(), [](std::string s) { return s.find("dev_flash_") == umax; }),
 		update_filenames.end());
 
-	std::string version_string = pup.get_file(0x100).to_string();
+	if (update_filenames.empty())
+	{
+		gui_log.error("Error while installing firmware: No dev_flash_* packages were found.");
+		critical(tr("Firmware installation failed: The provided file's contents are corrupted."));
+		return;
+	}
+
+	static constexpr std::string_view cur_version = "4.87";
+
+	std::string version_string;
+
+	if (fs::file version = pup.get_file(0x100))
+	{
+		version_string = version.to_string();
+	}
 
 	if (const usz version_pos = version_string.find('\n'); version_pos != umax)
 	{
 		version_string.erase(version_pos);
 	}
 
-	const std::string cur_version = "4.87";
+	if (version_string.empty())
+	{
+		gui_log.error("Error while installing firmware: No version data was found.");
+		critical(tr("Firmware installation failed: The provided file's contents are corrupted."));
+		return;
+	}
 
 	if (version_string < cur_version &&
-		QMessageBox::question(this, tr("RPCS3 Firmware Installer"), tr("Old firmware detected.\nThe newest firmware version is %1 and you are trying to install version %2\nContinue installation?").arg(qstr(cur_version), qstr(version_string)),
+		QMessageBox::question(this, tr("RPCS3 Firmware Installer"), tr("Old firmware detected.\nThe newest firmware version is %1 and you are trying to install version %2\nContinue installation?").arg(QString::fromUtf8(cur_version.data(), ::size32(cur_version)), qstr(version_string)),
 			QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes) == QMessageBox::No)
 	{
 		return;
@@ -1005,7 +1045,6 @@ void main_window::HandlePupInstallation(QString file_path)
 	}
 
 	update_files_f.close();
-	pup_f.close();
 
 	if (progress == update_filenames.size())
 	{