[ros-diffs] [pschweitzer] 68239: [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 pr...

Author: pschweitzer
Date: Mon Jun 22 17:27:47 2015
New Revision: 68239

URL: http://svn.reactos.org/svn/reactos?rev=68239&view=rev
Log:
[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

Modified:
    trunk/reactos/drivers/filesystems/cdfs/cdfs.h
    trunk/reactos/drivers/filesystems/cdfs/dirctl.c
    trunk/reactos/drivers/filesystems/cdfs/fcb.c
    trunk/reactos/drivers/filesystems/cdfs/misc.c

Modified: trunk/reactos/drivers/filesystems/cdfs/cdfs.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/cdfs/c…
==============================================================================
--- trunk/reactos/drivers/filesystems/cdfs/cdfs.h	[iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/cdfs/cdfs.h	[iso-8859-1] Mon Jun 22 17:27:47 2015
@@ -480,6 +480,10 @@
  PUNICODE_STRING LongName,
  PUNICODE_STRING ShortName);
 
+BOOLEAN
+CdfsIsRecordValid(IN PDEVICE_EXTENSION DeviceExt,
+                  IN PDIR_RECORD Record);
+
 /* rw.c */
 
 NTSTATUS

Modified: trunk/reactos/drivers/filesystems/cdfs/dirctl.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/cdfs/d…
==============================================================================
--- trunk/reactos/drivers/filesystems/cdfs/dirctl.c	[iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/cdfs/dirctl.c	[iso-8859-1] Mon Jun 22 17:27:47 2015
@@ -290,9 +290,8 @@
             return Status;
         }
 
-        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));
+        if (!CdfsIsRecordValid(DeviceExt, Record))
+        {
             RtlFreeUnicodeString(&FileToFindUpcase);
             CcUnpinData(Context);
             return STATUS_DISK_CORRUPT_ERROR;
@@ -301,14 +300,6 @@
         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;

Modified: trunk/reactos/drivers/filesystems/cdfs/fcb.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/cdfs/f…
==============================================================================
--- trunk/reactos/drivers/filesystems/cdfs/fcb.c	[iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/cdfs/fcb.c	[iso-8859-1] Mon Jun 22 17:27:47 2015
@@ -558,9 +558,8 @@
         DPRINT("RecordLength %u  ExtAttrRecordLength %u  NameLength %u\n",
             Record->RecordLength, Record->ExtAttrRecordLength,
Record->FileIdLength);
 
-        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));
+        if (!CdfsIsRecordValid(DeviceExt, Record))
+        {
             RtlFreeUnicodeString(&FileToFindUpcase);
             CcUnpinData(Context);
             return STATUS_DISK_CORRUPT_ERROR;
@@ -572,15 +571,6 @@
         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));
 

Modified: trunk/reactos/drivers/filesystems/cdfs/misc.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/cdfs/m…
==============================================================================
--- trunk/reactos/drivers/filesystems/cdfs/misc.c	[iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/cdfs/misc.c	[iso-8859-1] Mon Jun 22 17:27:47 2015
@@ -206,6 +206,55 @@
     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,