Filesystem: Check FST size to avoid reading beyond the buffer

Source/Core/DiscIO/FileSystemGCWii.cpp

@@ -21,17 +21,19 @@
 namespace DiscIO
 {
 // Set everything manually
-CFileInfoGCWii::CFileInfoGCWii(const u8* fst, bool wii, u32 index, u32 total_file_infos)
+CFileInfoGCWii::CFileInfoGCWii(const u8* fst, bool wii, u32 fst_size, u32 index, u32 total_file_infos)
 	: m_fst(fst),
 	m_wii(wii),
+	m_fst_size(fst_size),
 	m_index(index),
 	m_total_file_infos(total_file_infos)
 { }
 
 // For the root object only
-CFileInfoGCWii::CFileInfoGCWii(const u8* fst, bool wii)
+CFileInfoGCWii::CFileInfoGCWii(const u8* fst, bool wii, u32 fst_size)
 	: m_fst(fst),
 	m_wii(wii),
+	m_fst_size(fst_size),
 	m_index(0),
 	m_total_file_infos(GetSize())
 { }
@@ -40,14 +42,33 @@ CFileInfoGCWii::CFileInfoGCWii(const u8* fst, bool wii)
 CFileInfoGCWii::CFileInfoGCWii(const CFileInfoGCWii& file_info, u32 index)
 	: m_fst(file_info.m_fst),
 	m_wii(file_info.m_wii),
-	m_index(index),
+	m_fst_size(file_info.m_fst_size),
+	m_index(FixIndex(index)),
 	m_total_file_infos(file_info.m_total_file_infos)
 { }
 
 CFileInfoGCWii::~CFileInfoGCWii()
 {
 }
 
+
+u64 CFileInfoGCWii::GetFSTSize() const
+{
+	return (u64)m_fst_size << (m_wii ? 2 : 0);
+}
+
+u32 CFileInfoGCWii::FixIndex(u32 index) const
+{
+	if (GetFSTSize() / 0xC <= index)
+	{
+		// To avoid reading beyond the end of the FST later, the index is clamped if it's too large.
+		ERROR_LOG(DISCIO, "Index in file system is too large");
+		return (u32)(GetFSTSize() / 0xC - 1);
+	}
+
+	return index;
+}
+
 const u8* CFileInfoGCWii::GetAddress() const
 {
 	return m_fst + m_index * 0xC;
@@ -99,9 +120,18 @@ bool CFileInfoGCWii::IsDirectory() const { return (GetNameOffset() & 0xFF000000)
 u32 CFileInfoGCWii::GetTotalChildren() const { return GetSize() - (m_index + 1); }
 std::string CFileInfoGCWii::GetName() const
 {
+	const u8* name_offset = m_fst + m_total_file_infos * 0xC + (GetNameOffset() & 0xFFFFFF);
+
+	if (name_offset >= m_fst + GetFSTSize())
+	{
+		// To avoid reading beyond the end of the FST, nothing is read if the name offset is too large.
+		ERROR_LOG(DISCIO, "Name offset in file system is too large");
+		return "";
+	}
+
 	// TODO: Should we really always use SHIFT-JIS?
 	// Some names in Pikmin (NTSC-U) don't make sense without it, but is it correct?
-	return SHIFTJISToUTF8((char*)(m_fst + m_total_file_infos * 0xC + (GetNameOffset() & 0xFFFFFF)));
+	return SHIFTJISToUTF8((char*)name_offset);
 }
 
 std::string CFileInfoGCWii::GetPath() const
@@ -145,7 +175,7 @@ CFileSystemGCWii::CFileSystemGCWii(const IVolume *_rVolume)
 	: IFileSystem(_rVolume)
 	, m_Valid(false)
 	, m_Wii(false)
-	, m_root(nullptr, false, 0, 0)
+	, m_root(nullptr, false, 0, 0, 0)
 {
 	// Check if this is a GameCube or Wii disc
 	if (m_rVolume->Read32(0x18, false) == 0x5D1C9EA3)
@@ -156,19 +186,20 @@ CFileSystemGCWii::CFileSystemGCWii(const IVolume *_rVolume)
 		return;
 
 	// read the whole FST
-	u64 FSTOffset = static_cast<u64>(m_rVolume->Read32(0x424, m_Wii)) << GetOffsetShift();
-	u64 FSTSize = static_cast<u64>(m_rVolume->Read32(0x428, m_Wii)) << GetOffsetShift();
-	if (FSTSize < 0xC)
+	u64 fst_offset = static_cast<u64>(m_rVolume->Read32(0x424, m_Wii)) << GetOffsetShift();
+	u32 raw_fst_size = m_rVolume->Read32(0x428, m_Wii);
+	u64 fst_size = static_cast<u64>(m_rVolume->Read32(0x428, m_Wii)) << GetOffsetShift();
+	if (fst_size < 0xC)
 		return;
-	m_file_system_table.resize(FSTSize + 1);
-	if (!m_rVolume->Read(FSTOffset, FSTSize, m_file_system_table.data(), m_Wii))
+	m_file_system_table.resize(fst_size + 1);
+	if (!m_rVolume->Read(fst_offset, fst_size, m_file_system_table.data(), m_Wii))
 		return;
 
 	// Just in case the last string in the name table isn't null terminated
-	m_file_system_table[FSTSize - 1] = 0;
+	m_file_system_table[fst_size - 1] = 0;
 
 	// Create the root object
-	m_root = CFileInfoGCWii(m_file_system_table.data(), m_Wii);
+	m_root = CFileInfoGCWii(m_file_system_table.data(), m_Wii, raw_fst_size);
 
 	if (!m_root.IsDirectory())
 		return;

Source/Core/DiscIO/FileSystemGCWii.h

@@ -22,9 +22,9 @@ class CFileInfoGCWii : public IFileInfo
 	// None of the constructors take ownership of FST pointers
 
 	// Set everything manually
-	CFileInfoGCWii(const u8* fst, bool wii, u32 index, u32 total_file_infos);
+	CFileInfoGCWii(const u8* fst, bool wii, u32 fst_size, u32 index, u32 total_file_infos);
 	// For the root object only
-	CFileInfoGCWii(const u8* fst, bool wii);
+	CFileInfoGCWii(const u8* fst, bool wii, u32 fst_size);
 	// Copies another object
 	CFileInfoGCWii(const CFileInfoGCWii& file_info) = default;
 	// Copies data that is common to the whole file system
@@ -55,9 +55,12 @@ class CFileInfoGCWii : public IFileInfo
 	u32 GetNameOffset() const;
 	// For directories, returns the index of the parent directory (or 0 if root)
 	u32 GetRawOffset() const;
+	u64 GetFSTSize() const;
+	u32 FixIndex(u32 index) const;
 
 	const u8* m_fst;
 	bool m_wii;
+	u32 m_fst_size;
 	u32 m_index;
 	u32 m_total_file_infos;
 };