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

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828

The commit refines the boundary checks for file/path name string to
prevent possible buffer overrun.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index eb7d79692bf36892ec01390e9f5da2f7b09e26e3..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


@@ -457,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


@@ -469,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


@@ -735,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


@@ -811,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


@@ -830,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


@@ -840,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

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 9048a18f6418fd69ec028e2995f12bc778eaf49f..b3ac673cac4952518d65cbe6635566abc2a605f4 100644 (file)
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2,6 +2,7 @@
   Handle on-disk format and volume structures in UDF/ECMA-167 file systems.\r
 \r
   Copyright (C) 2014-2017 Paulo Alcantara <pcacjr@zytor.com>\r
   Handle on-disk format and volume structures in 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


@@ -1466,7 +1467,7 @@ InternalFindFile (
         break;\r
       }\r
     } else {\r
         break;\r
       }\r
     } else {\r

-      Status = GetFileNameFromFid (FileIdentifierDesc, FoundFileName);\r
+      Status = GetFileNameFromFid (FileIdentifierDesc, ARRAY_SIZE (FoundFileName), FoundFileName);\r

       if (EFI_ERROR (Status)) {\r
         break;\r
       }\r
       if (EFI_ERROR (Status)) {\r
         break;\r
       }\r


@@ -1763,6 +1764,11 @@ FindFile (
   while (*FilePath != L'\0') {\r
     FileNamePointer = FileName;\r
     while (*FilePath != L'\0' && *FilePath != L'\\') {\r
   while (*FilePath != L'\0') {\r
     FileNamePointer = FileName;\r
     while (*FilePath != L'\0' && *FilePath != L'\\') {\r

+      if ((((UINTN)FileNamePointer - (UINTN)FileName) / sizeof (CHAR16)) >=\r
+          (ARRAY_SIZE (FileName) - 1)) {\r
+        return EFI_NOT_FOUND;\r
+      }\r
+\r

       *FileNamePointer++ = *FilePath++;\r
     }\r
 \r
       *FileNamePointer++ = *FilePath++;\r
     }\r
 \r


@@ -1954,22 +1960,38 @@ ReadDirectoryEntry (
   Get a filename (encoded in OSTA-compressed format) from a File Identifier\r
   Descriptor on an UDF volume.\r
 \r
   Get a filename (encoded in OSTA-compressed format) from a File Identifier\r
   Descriptor on an UDF volume.\r
 \r

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

   @param[in]   FileIdentifierDesc  File Identifier Descriptor pointer.\r
   @param[in]   FileIdentifierDesc  File Identifier Descriptor pointer.\r

+  @param[in]   CharMax             The maximum number of FileName Unicode char,\r
+                                   including terminating null char.\r

   @param[out]  FileName            Decoded filename.\r
 \r
   @retval EFI_SUCCESS           Filename decoded and read.\r
   @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.\r
   @param[out]  FileName            Decoded filename.\r
 \r
   @retval EFI_SUCCESS           Filename decoded and read.\r
   @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.\r

+  @retval EFI_BUFFER_TOO_SMALL  The string buffer FileName cannot hold the\r
+                                decoded filename.\r

 **/\r
 EFI_STATUS\r
 GetFileNameFromFid (\r
   IN   UDF_FILE_IDENTIFIER_DESCRIPTOR  *FileIdentifierDesc,\r
 **/\r
 EFI_STATUS\r
 GetFileNameFromFid (\r
   IN   UDF_FILE_IDENTIFIER_DESCRIPTOR  *FileIdentifierDesc,\r

+  IN   UINTN                           CharMax,\r

   OUT  CHAR16                          *FileName\r
   )\r
 {\r
   OUT  CHAR16                          *FileName\r
   )\r
 {\r

-  UINT8 *OstaCompressed;\r
-  UINT8 CompressionId;\r
-  UINT8 Length;\r
-  UINTN Index;\r
+  UINT8  *OstaCompressed;\r
+  UINT8  CompressionId;\r
+  UINT8  Length;\r
+  UINTN  Index;\r
+  CHAR16 *FileNameBak;\r
+\r
+  if (CharMax == 0) {\r
+    return EFI_BUFFER_TOO_SMALL;\r
+  }\r

 \r
   OstaCompressed =\r
     (UINT8 *)(\r
 \r
   OstaCompressed =\r
     (UINT8 *)(\r


@@ -1982,10 +2004,22 @@ GetFileNameFromFid (
     return EFI_VOLUME_CORRUPTED;\r
   }\r
 \r
     return EFI_VOLUME_CORRUPTED;\r
   }\r
 \r

+  FileNameBak = FileName;\r
+\r

   //\r
   // Decode filename.\r
   //\r
   Length = FileIdentifierDesc->LengthOfFileIdentifier;\r
   //\r
   // Decode filename.\r
   //\r
   Length = FileIdentifierDesc->LengthOfFileIdentifier;\r

+  if (CompressionId == 16) {\r
+    if (((UINTN)Length >> 1) > CharMax) {\r
+      return EFI_BUFFER_TOO_SMALL;\r
+    }\r
+  } else {\r
+    if ((Length != 0) && ((UINTN)Length - 1 > CharMax)) {\r
+      return EFI_BUFFER_TOO_SMALL;\r
+    }\r
+  }\r
+\r

   for (Index = 1; Index < Length; Index++) {\r
     if (CompressionId == 16) {\r
       *FileName = OstaCompressed[Index++] << 8;\r
   for (Index = 1; Index < Length; Index++) {\r
     if (CompressionId == 16) {\r
       *FileName = OstaCompressed[Index++] << 8;\r


@@ -2000,7 +2034,11 @@ GetFileNameFromFid (
     FileName++;\r
   }\r
 \r
     FileName++;\r
   }\r
 \r

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

 \r
   return EFI_SUCCESS;\r
 }\r
 \r
   return EFI_SUCCESS;\r
 }\r


@@ -2008,6 +2046,12 @@ GetFileNameFromFid (
 /**\r
   Resolve a symlink file on an UDF volume.\r
 \r
 /**\r
   Resolve a symlink file on an UDF volume.\r
 \r

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

   @param[in]   BlockIo        BlockIo interface.\r
   @param[in]   DiskIo         DiskIo interface.\r
   @param[in]   Volume         UDF volume information structure.\r
   @param[in]   BlockIo        BlockIo interface.\r
   @param[in]   DiskIo         DiskIo interface.\r
   @param[in]   Volume         UDF volume information structure.\r


@@ -2136,6 +2180,9 @@ ResolveSymlink (
                           Index) << 8;\r
           Index++;\r
         } else {\r
                           Index) << 8;\r
           Index++;\r
         } else {\r

+          if (Index > ARRAY_SIZE (FileName)) {\r
+            return EFI_UNSUPPORTED;\r
+          }\r

           *Char = 0;\r
         }\r
 \r
           *Char = 0;\r
         }\r
 \r


@@ -2146,7 +2193,11 @@ ResolveSymlink (
         Char++;\r
       }\r
 \r
         Char++;\r
       }\r
 \r

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

       break;\r
     }\r
 \r
       break;\r
     }\r
 \r

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index d441539d162de92e5a56b21d0519b870e639ca1b..9b82441e720b349de9835138a16d188567802c10 100644 (file)
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -2,6 +2,7 @@
   UDF/ECMA-167 file system driver.\r
 \r
   Copyright (C) 2014-2017 Paulo Alcantara <pcacjr@zytor.com>\r
   UDF/ECMA-167 file system driver.\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


@@ -559,9 +560,16 @@ 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  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 buffer.\r
+  @param  BufferSize      On input size of buffer, on output amount of data in\r
+                          buffer.\r

   @param  Buffer          The buffer to return data.\r
 \r
   @retval EFI_SUCCESS          Data was returned.\r
   @param  Buffer          The buffer to return data.\r
 \r
   @retval EFI_SUCCESS          Data was returned.\r


@@ -571,7 +579,8 @@ UdfSetPosition (
   @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.\r
   @retval EFI_WRITE_PROTECTED  The device is write protected.\r
   @retval EFI_ACCESS_DENIED    The file was open for read only.\r
   @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.\r
   @retval EFI_WRITE_PROTECTED  The device is write protected.\r
   @retval EFI_ACCESS_DENIED    The file was open for read only.\r

-  @retval EFI_BUFFER_TOO_SMALL Buffer was too small; required size returned in BufferSize.\r
+  @retval EFI_BUFFER_TOO_SMALL Buffer was too small; required size returned in\r
+                               BufferSize.\r

 \r
 **/\r
 EFI_STATUS\r
 \r
 **/\r
 EFI_STATUS\r


@@ -769,21 +778,38 @@ ReadDirectoryEntry (
   Get a filename (encoded in OSTA-compressed format) from a File Identifier\r
   Descriptor on an UDF volume.\r
 \r
   Get a filename (encoded in OSTA-compressed format) from a File Identifier\r
   Descriptor on an UDF volume.\r
 \r

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

   @param[in]   FileIdentifierDesc  File Identifier Descriptor pointer.\r
   @param[in]   FileIdentifierDesc  File Identifier Descriptor pointer.\r

+  @param[in]   CharMax             The maximum number of FileName Unicode char,\r
+                                   including terminating null char.\r

   @param[out]  FileName            Decoded filename.\r
 \r
   @retval EFI_SUCCESS           Filename decoded and read.\r
   @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.\r
   @param[out]  FileName            Decoded filename.\r
 \r
   @retval EFI_SUCCESS           Filename decoded and read.\r
   @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.\r

+  @retval EFI_BUFFER_TOO_SMALL  The string buffer FileName cannot hold the\r
+                                decoded filename.\r

 **/\r
 EFI_STATUS\r
 GetFileNameFromFid (\r
   IN   UDF_FILE_IDENTIFIER_DESCRIPTOR  *FileIdentifierDesc,\r
 **/\r
 EFI_STATUS\r
 GetFileNameFromFid (\r
   IN   UDF_FILE_IDENTIFIER_DESCRIPTOR  *FileIdentifierDesc,\r

+  IN   UINTN                           CharMax,\r

   OUT  CHAR16                          *FileName\r
   );\r
 \r
 /**\r
   Resolve a symlink file on an UDF volume.\r
 \r
   OUT  CHAR16                          *FileName\r
   );\r
 \r
 /**\r
   Resolve a symlink file on an UDF volume.\r
 \r

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

   @param[in]   BlockIo        BlockIo interface.\r
   @param[in]   DiskIo         DiskIo interface.\r
   @param[in]   Volume         UDF volume information structure.\r
   @param[in]   BlockIo        BlockIo interface.\r
   @param[in]   DiskIo         DiskIo interface.\r
   @param[in]   Volume         UDF volume information structure.\r