[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

diff --git a/reactos/drivers/filesystems/cdfs/cdfs.h b/reactos/drivers/filesystems/cdfs/cdfs.h
index 7f4162e..d07ba06 100644 (file)
--- 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 7bfbc2d..0393fe7 100644 (file)
--- 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 5986b91..7d0b27d 100644 (file)
--- 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 a4f0d38..56ec63f 100644 (file)
--- 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,