1 files changed, 67 insertions, 36 deletions

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<u64>(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<const char*>(name));
+  return SHIFTJISToUTF8(reinterpret_cast<const char*>(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<const FileInfoGCWii&>(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()