diff --git a/Source/Core/DiscIO/FileSystemGCWii.cpp b/Source/Core/DiscIO/FileSystemGCWii.cpp index 8546e4d4d1..8c3ce41b5b 100644 --- a/Source/Core/DiscIO/FileSystemGCWii.cpp +++ b/Source/Core/DiscIO/FileSystemGCWii.cpp @@ -88,18 +88,7 @@ u32 FileInfoGCWii::Get(EntryProperty entry_property) const u32 FileInfoGCWii::GetSize() const { - u32 result = Get(EntryProperty::FILE_SIZE); - - if (IsDirectory() && result <= m_index) - { - // For directories, GetSize is supposed to return the index of the next entry. - // If a file system is malformed and instead has an index that isn't after this one, - // we act as if the directory is empty to avoid strange behavior. - ERROR_LOG(DISCIO, "Invalid folder end in file system"); - return m_index + 1; - } - - return result; + return Get(EntryProperty::FILE_SIZE); } u64 FileInfoGCWii::GetOffset() const @@ -114,16 +103,20 @@ bool FileInfoGCWii::IsDirectory() const u32 FileInfoGCWii::GetTotalChildren() const { - return GetSize() - (m_index + 1); + return Get(EntryProperty::FILE_SIZE) - (m_index + 1); +} + +u64 FileInfoGCWii::GetNameOffset() const +{ + return static_cast(FST_ENTRY_SIZE) * m_total_file_infos + + (Get(EntryProperty::NAME_OFFSET) & 0xFFFFFF); } std::string FileInfoGCWii::GetName() const { // TODO: Should we really always use SHIFT-JIS? // Some names in Pikmin (NTSC-U) don't make sense without it, but is it correct? - u32 name_offset = Get(EntryProperty::NAME_OFFSET) & 0xFFFFFF; - const u8* name = m_fst + FST_ENTRY_SIZE * m_total_file_infos + name_offset; - return SHIFTJISToUTF8(reinterpret_cast(name)); + return SHIFTJISToUTF8(reinterpret_cast(m_fst + GetNameOffset())); } std::string FileInfoGCWii::GetPath() const @@ -135,37 +128,63 @@ std::string FileInfoGCWii::GetPath() const if (IsDirectory()) { u32 parent_directory_index = Get(EntryProperty::FILE_OFFSET); - if (parent_directory_index >= m_index) - { - // The index of the parent directory is supposed to be smaller than - // the current index. If an FST is malformed and breaks that rule, - // there's a risk that parent directory pointers form a loop. - // To avoid stack overflows, this method returns. - ERROR_LOG(DISCIO, "Invalid parent offset in file system"); - return ""; - } return FileInfoGCWii(*this, parent_directory_index).GetPath() + GetName() + "/"; } else { // The parent directory can be found by searching backwards - // for a directory that contains this file. - + // for a directory that contains this file. The search cannot fail, + // because the root directory at index 0 contains all files. FileInfoGCWii potential_parent(*this, m_index - 1); - while (!(potential_parent.IsDirectory() && potential_parent.GetSize() > m_index)) + while (!(potential_parent.IsDirectory() && + potential_parent.Get(EntryProperty::FILE_SIZE) > m_index)) { - if (potential_parent.m_index == 0) - { - // This can happen if an FST has a root with a size that's too small - ERROR_LOG(DISCIO, "The parent of %s couldn't be found", GetName().c_str()); - return ""; - } potential_parent = FileInfoGCWii(*this, potential_parent.m_index - 1); } return potential_parent.GetPath() + GetName(); } } +bool FileInfoGCWii::IsValid(u64 fst_size, const FileInfoGCWii& parent_directory) const +{ + if (GetNameOffset() >= fst_size) + { + ERROR_LOG(DISCIO, "Impossibly large name offset in file system"); + return false; + } + + if (IsDirectory()) + { + if (Get(EntryProperty::FILE_OFFSET) != parent_directory.m_index) + { + ERROR_LOG(DISCIO, "Incorrect parent offset in file system"); + return false; + } + + u32 size = Get(EntryProperty::FILE_SIZE); + + if (size <= m_index) + { + ERROR_LOG(DISCIO, "Impossibly small directory size in file system"); + return false; + } + + if (size > parent_directory.Get(EntryProperty::FILE_SIZE)) + { + ERROR_LOG(DISCIO, "Impossibly large directory size in file system"); + return false; + } + + for (const FileInfo& child : *this) + { + if (!static_cast(child).IsValid(fst_size, *this)) + return false; + } + } + + return true; +} + FileSystemGCWii::FileSystemGCWii(const Volume* _rVolume, const Partition& partition) : FileSystem(_rVolume, partition), m_Valid(false), m_offset_shift(0), m_root(nullptr, 0, 0, 0) { @@ -217,8 +236,20 @@ FileSystemGCWii::FileSystemGCWii(const Volume* _rVolume, const Partition& partit return; } - // If we haven't returned yet, everything succeeded - m_Valid = true; + if (FST_ENTRY_SIZE * m_root.GetSize() > fst_size) + { + ERROR_LOG(DISCIO, "File system has too many entries for its size"); + return; + } + + // If the FST's final byte isn't 0, CFileInfoGCWii::GetName() can read past the end + if (m_file_system_table[fst_size - 1] != 0) + { + ERROR_LOG(DISCIO, "File system does not end with a null byte"); + return; + } + + m_Valid = m_root.IsValid(fst_size, m_root); } FileSystemGCWii::~FileSystemGCWii() diff --git a/Source/Core/DiscIO/FileSystemGCWii.h b/Source/Core/DiscIO/FileSystemGCWii.h index 158aa3e420..52013277b7 100644 --- a/Source/Core/DiscIO/FileSystemGCWii.h +++ b/Source/Core/DiscIO/FileSystemGCWii.h @@ -46,6 +46,8 @@ public: std::string GetName() const override; std::string GetPath() const override; + bool IsValid(u64 fst_size, const FileInfoGCWii& parent_directory) const; + protected: uintptr_t GetAddress() const override; FileInfo& operator++() override; @@ -71,6 +73,8 @@ private: // Returns one of the three properties of this FST entry. // Read the comments in EntryProperty for details. u32 Get(EntryProperty entry_property) const; + // Returns the name offset, excluding the directory identification byte + u64 GetNameOffset() const; const u8* m_fst; u8 m_offset_shift;