From 8329f09d38fa4a7c82d594a4e419532f5d7fceda Mon Sep 17 00:00:00 2001
From: Pierre Schweitzer <pierre@reactos.org>
Date: Mon, 22 Jun 2015 17:27:47 +0000
Subject: [PATCH] [CDFS] Revamp r68233: - Don't duplicate code, implement
 checks in a helper function - When checking name content, do it earlier for
 better performances - Add extra checks to prevent a potential buffer overflow
 in case of Joliet names with illformed entries

CORE-9254

svn path=/trunk/; revision=68239
---
 reactos/drivers/filesystems/cdfs/cdfs.h   |  4 ++
 reactos/drivers/filesystems/cdfs/dirctl.c | 11 +----
 reactos/drivers/filesystems/cdfs/fcb.c    | 12 +-----
 reactos/drivers/filesystems/cdfs/misc.c   | 49 +++++++++++++++++++++++
 4 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/reactos/drivers/filesystems/cdfs/cdfs.h b/reactos/drivers/filesystems/cdfs/cdfs.h
index 7f4162ecca7..d07ba06a7d9 100644
--- a/reactos/drivers/filesystems/cdfs/cdfs.h
+++ b/reactos/drivers/filesystems/cdfs/cdfs.h
@@ -480,6 +480,10 @@ CdfsShortNameCacheGet
  PUNICODE_STRING LongName,
  PUNICODE_STRING ShortName);
 
+BOOLEAN
+CdfsIsRecordValid(IN PDEVICE_EXTENSION DeviceExt,
+                  IN PDIR_RECORD Record);
+
 /* rw.c */
 
 NTSTATUS
diff --git a/reactos/drivers/filesystems/cdfs/dirctl.c b/reactos/drivers/filesystems/cdfs/dirctl.c
index 7bfbc2de6e6..0393fe7f8ed 100644
--- a/reactos/drivers/filesystems/cdfs/dirctl.c
+++ b/reactos/drivers/filesystems/cdfs/dirctl.c
@@ -290,9 +290,8 @@ CdfsFindFile(PDEVICE_EXTENSION DeviceExt,
             return Status;
         }
 
-        if (Record->RecordLength < Record->FileIdLength + FIELD_OFFSET(DIR_RECORD, FileId))
+        if (!CdfsIsRecordValid(DeviceExt, Record))
         {
-            DPRINT1("Found corrupted entry! %u - %u\n", Record->RecordLength, Record->FileIdLength + FIELD_OFFSET(DIR_RECORD, FileId));
             RtlFreeUnicodeString(&FileToFindUpcase);
             CcUnpinData(Context);
             return STATUS_DISK_CORRUPT_ERROR;
@@ -301,14 +300,6 @@ CdfsFindFile(PDEVICE_EXTENSION DeviceExt,
         DPRINT("Name '%S'\n", name);
 
         RtlInitUnicodeString(&LongName, name);
-        /* Was the entry degenerated? */
-        if (LongName.Length < sizeof(WCHAR))
-        {
-            DPRINT1("Found entry with invalid name!\n");
-            RtlFreeUnicodeString(&FileToFindUpcase);
-            CcUnpinData(Context);
-            return STATUS_DISK_CORRUPT_ERROR;
-        }
 
         ShortName.Length = 0;
         ShortName.MaximumLength = 26;
diff --git a/reactos/drivers/filesystems/cdfs/fcb.c b/reactos/drivers/filesystems/cdfs/fcb.c
index 5986b9170f5..7d0b27dde53 100644
--- a/reactos/drivers/filesystems/cdfs/fcb.c
+++ b/reactos/drivers/filesystems/cdfs/fcb.c
@@ -558,9 +558,8 @@ CdfsDirFindFile(PDEVICE_EXTENSION DeviceExt,
         DPRINT("RecordLength %u  ExtAttrRecordLength %u  NameLength %u\n",
             Record->RecordLength, Record->ExtAttrRecordLength, Record->FileIdLength);
 
-        if (Record->RecordLength < Record->FileIdLength + FIELD_OFFSET(DIR_RECORD, FileId))
+        if (!CdfsIsRecordValid(DeviceExt, Record))
         {
-            DPRINT1("Found corrupted entry! %u - %u\n", Record->RecordLength, Record->FileIdLength + FIELD_OFFSET(DIR_RECORD, FileId));
             RtlFreeUnicodeString(&FileToFindUpcase);
             CcUnpinData(Context);
             return STATUS_DISK_CORRUPT_ERROR;
@@ -572,15 +571,6 @@ CdfsDirFindFile(PDEVICE_EXTENSION DeviceExt,
         DPRINT ("Offset %lu\n", Offset);
 
         RtlInitUnicodeString(&LongName, Name);
-        /* Was the entry degenerated? */
-        if (LongName.Length < sizeof(WCHAR))
-        {
-            DPRINT1("Found entry with invalid name!\n");
-            RtlFreeUnicodeString(&FileToFindUpcase);
-            CcUnpinData(Context);
-            return STATUS_DISK_CORRUPT_ERROR;
-        }
-
         RtlInitEmptyUnicodeString(&ShortName, ShortNameBuffer, sizeof(ShortNameBuffer));
         RtlZeroMemory(ShortNameBuffer, sizeof(ShortNameBuffer));
 
diff --git a/reactos/drivers/filesystems/cdfs/misc.c b/reactos/drivers/filesystems/cdfs/misc.c
index a4f0d380c38..56ec63fd217 100644
--- a/reactos/drivers/filesystems/cdfs/misc.c
+++ b/reactos/drivers/filesystems/cdfs/misc.c
@@ -206,6 +206,55 @@ CdfsIsNameLegalDOS8Dot3(IN UNICODE_STRING FileName
     return FsRtlIsFatDbcsLegal(DbcsName, FALSE, FALSE, FALSE);
 }
 
+BOOLEAN
+CdfsIsRecordValid(IN PDEVICE_EXTENSION DeviceExt,
+                  IN PDIR_RECORD Record)
+{
+    if (Record->RecordLength < Record->FileIdLength + FIELD_OFFSET(DIR_RECORD, FileId))
+    {
+        DPRINT1("Found corrupted entry! %u - %u\n", Record->RecordLength, Record->FileIdLength + FIELD_OFFSET(DIR_RECORD, FileId));
+        return FALSE;
+    }
+
+    if (Record->FileIdLength == 0)
+    {
+        DPRINT1("Found corrupted entry (null size)!\n");
+        return FALSE;
+    }
+
+    if (DeviceExt->CdInfo.JolietLevel == 0)
+    {
+        if (Record->FileId[0] == ANSI_NULL && Record->FileIdLength != 1)
+        {
+            DPRINT1("Found corrupted entry!\n");
+            return FALSE;
+        }
+    }
+    else
+    {
+        if (Record->FileIdLength & 1 && Record->FileIdLength != 1)
+        {
+            DPRINT1("Found corrupted entry! %u\n", Record->FileIdLength);
+            return FALSE;
+        }
+
+        if (Record->FileIdLength == 1 && Record->FileId[0] != 0 &&  Record->FileId[0] != 1)
+        {
+            DPRINT1("Found corrupted entry! %c\n", Record->FileId[0]);
+            DPRINT1("%wc\n", ((PWSTR)Record->FileId)[0]);
+            return FALSE;
+        }
+
+        if (((PWSTR)Record->FileId)[0] == UNICODE_NULL)
+        {
+            DPRINT1("Found corrupted entry!\n");
+            return FALSE;
+        }
+    }
+
+    return TRUE;
+}
+
 VOID
 CdfsShortNameCacheGet
 (PFCB DirectoryFcb, 
-- 
2.17.1