MdeModulePkg/UdfDxe: Refine boundary checks for file/path name string

[mirror_edk2.git] / MdeModulePkg / Universal / Disk / UdfDxe / File.c

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 6f07bf2066dced6e3836ecfa659b3e984c71f063..4aa0594f1a9e9d87f3252382b91714bb75efa611 100644 (file)
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -2,6 +2,7 @@
   Handle operations in files and directories from UDF/ECMA-167 file systems.\r
 \r
   Copyright (C) 2014-2017 Paulo Alcantara <pcacjr@zytor.com>\r
   Handle operations in files and directories from UDF/ECMA-167 file systems.\r
 \r
   Copyright (C) 2014-2017 Paulo Alcantara <pcacjr@zytor.com>\r

+  Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>\r

 \r
   This program and the accompanying materials are licensed and made available\r
   under the terms and conditions of the BSD License which accompanies this\r
 \r
   This program and the accompanying materials are licensed and made available\r
   under the terms and conditions of the BSD License which accompanies this\r


@@ -248,7 +249,7 @@ UdfOpen (
     FileName = TempFileName + 1;\r
   }\r
 \r
     FileName = TempFileName + 1;\r
   }\r
 \r

-  StrCpyS (NewPrivFileData->FileName, UDF_PATH_LENGTH, FileName);\r
+  StrCpyS (NewPrivFileData->FileName, UDF_FILENAME_LENGTH, FileName);\r

 \r
   Status = GetFileSize (\r
     PrivFsData->BlockIo,\r
 \r
   Status = GetFileSize (\r
     PrivFsData->BlockIo,\r


@@ -257,8 +258,12 @@ UdfOpen (
     &NewPrivFileData->File,\r
     &NewPrivFileData->FileSize\r
     );\r
     &NewPrivFileData->File,\r
     &NewPrivFileData->FileSize\r
     );\r

-  ASSERT_EFI_ERROR (Status);\r

   if (EFI_ERROR (Status)) {\r
   if (EFI_ERROR (Status)) {\r

+    DEBUG ((\r
+      DEBUG_ERROR,\r
+      "%a: GetFileSize() fails with status - %r.\n",\r
+      __FUNCTION__, Status\r
+      ));\r

     goto Error_Get_File_Size;\r
   }\r
 \r
     goto Error_Get_File_Size;\r
   }\r
 \r


@@ -408,6 +413,15 @@ UdfRead (
 \r
         goto Done;\r
       }\r
 \r
         goto Done;\r
       }\r

+      //\r
+      // After calling function ReadDirectoryEntry(), if 'NewFileIdentifierDesc'\r
+      // is NULL, then the 'Status' must be EFI_OUT_OF_RESOURCES. Hence, if the\r
+      // code reaches here, 'NewFileIdentifierDesc' must be not NULL.\r
+      //\r
+      // The ASSERT here is for addressing a false positive NULL pointer\r
+      // dereference issue raised from static analysis.\r
+      //\r
+      ASSERT (NewFileIdentifierDesc != NULL);\r

 \r
       if (!IS_FID_PARENT_FILE (NewFileIdentifierDesc)) {\r
         break;\r
 \r
       if (!IS_FID_PARENT_FILE (NewFileIdentifierDesc)) {\r
         break;\r


@@ -444,7 +458,7 @@ UdfRead (
       FreePool ((VOID *)NewFileEntryData);\r
       NewFileEntryData = FoundFile.FileEntry;\r
 \r
       FreePool ((VOID *)NewFileEntryData);\r
       NewFileEntryData = FoundFile.FileEntry;\r
 \r

-      Status = GetFileNameFromFid (NewFileIdentifierDesc, FileName);\r
+      Status = GetFileNameFromFid (NewFileIdentifierDesc, ARRAY_SIZE (FileName), FileName);\r

       if (EFI_ERROR (Status)) {\r
         FreePool ((VOID *)FoundFile.FileIdentifierDesc);\r
         goto Error_Get_FileName;\r
       if (EFI_ERROR (Status)) {\r
         FreePool ((VOID *)FoundFile.FileIdentifierDesc);\r
         goto Error_Get_FileName;\r


@@ -456,7 +470,7 @@ UdfRead (
       FoundFile.FileIdentifierDesc  = NewFileIdentifierDesc;\r
       FoundFile.FileEntry           = NewFileEntryData;\r
 \r
       FoundFile.FileIdentifierDesc  = NewFileIdentifierDesc;\r
       FoundFile.FileEntry           = NewFileEntryData;\r
 \r

-      Status = GetFileNameFromFid (FoundFile.FileIdentifierDesc, FileName);\r
+      Status = GetFileNameFromFid (FoundFile.FileIdentifierDesc, ARRAY_SIZE (FileName), FileName);\r

       if (EFI_ERROR (Status)) {\r
         goto Error_Get_FileName;\r
       }\r
       if (EFI_ERROR (Status)) {\r
         goto Error_Get_FileName;\r
       }\r


@@ -487,6 +501,10 @@ UdfRead (
     PrivFileData->FilePosition++;\r
     Status = EFI_SUCCESS;\r
   } else if (IS_FID_DELETED_FILE (Parent->FileIdentifierDesc)) {\r
     PrivFileData->FilePosition++;\r
     Status = EFI_SUCCESS;\r
   } else if (IS_FID_DELETED_FILE (Parent->FileIdentifierDesc)) {\r

+    //\r
+    // Code should never reach here.\r
+    //\r
+    ASSERT (FALSE);\r

     Status = EFI_DEVICE_ERROR;\r
   }\r
 \r
     Status = EFI_DEVICE_ERROR;\r
   }\r
 \r


@@ -718,6 +736,12 @@ UdfSetPosition (
 /**\r
   Get information about a file.\r
 \r
 /**\r
   Get information about a file.\r
 \r

+  @attention This is boundary function that may receive untrusted input.\r
+  @attention The input is from FileSystem.\r
+\r
+  The File Set Descriptor is external input, so this routine will do basic\r
+  validation for File Set Descriptor and report status.\r
+\r

   @param  This            Protocol instance pointer.\r
   @param  InformationType Type of information to return in Buffer.\r
   @param  BufferSize      On input size of buffer, on output amount of data in\r
   @param  This            Protocol instance pointer.\r
   @param  InformationType Type of information to return in Buffer.\r
   @param  BufferSize      On input size of buffer, on output amount of data in\r


@@ -794,6 +818,10 @@ UdfGetInfo (
         *String = *(UINT8 *)(OstaCompressed + Index) << 8;\r
         Index++;\r
       } else {\r
         *String = *(UINT8 *)(OstaCompressed + Index) << 8;\r
         Index++;\r
       } else {\r

+        if (Index > ARRAY_SIZE (VolumeLabel)) {\r
+          return EFI_VOLUME_CORRUPTED;\r
+        }\r
+\r

         *String = 0;\r
       }\r
 \r
         *String = 0;\r
       }\r
 \r


@@ -813,7 +841,11 @@ UdfGetInfo (
       String++;\r
     }\r
 \r
       String++;\r
     }\r
 \r

-    *String = L'\0';\r
+    Index = ((UINTN)String - (UINTN)VolumeLabel) / sizeof (CHAR16);\r
+    if (Index > ARRAY_SIZE (VolumeLabel) - 1) {\r
+      Index = ARRAY_SIZE (VolumeLabel) - 1;\r
+    }\r
+    VolumeLabel[Index] = L'\0';\r

 \r
     FileSystemInfoLength = StrSize (VolumeLabel) +\r
                            sizeof (EFI_FILE_SYSTEM_INFO);\r
 \r
     FileSystemInfoLength = StrSize (VolumeLabel) +\r
                            sizeof (EFI_FILE_SYSTEM_INFO);\r


@@ -823,8 +855,11 @@ UdfGetInfo (
     }\r
 \r
     FileSystemInfo = (EFI_FILE_SYSTEM_INFO *)Buffer;\r
     }\r
 \r
     FileSystemInfo = (EFI_FILE_SYSTEM_INFO *)Buffer;\r

-    StrCpyS (FileSystemInfo->VolumeLabel, ARRAY_SIZE (VolumeLabel),\r
-             VolumeLabel);\r
+    StrCpyS (\r
+      FileSystemInfo->VolumeLabel,\r
+      (*BufferSize - OFFSET_OF (EFI_FILE_SYSTEM_INFO, VolumeLabel)) / sizeof (CHAR16),\r
+      VolumeLabel\r
+      );\r

     Status = GetVolumeSize (\r
       PrivFsData->BlockIo,\r
       PrivFsData->DiskIo,\r
     Status = GetVolumeSize (\r
       PrivFsData->BlockIo,\r
       PrivFsData->DiskIo,\r