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
+  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
@@ -248,7 +249,7 @@ UdfOpen (
     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
@@ -457,7 +458,7 @@ UdfRead (
       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
@@ -469,7 +470,7 @@ UdfRead (
       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
@@ -735,6 +736,12 @@ UdfSetPosition (
 /**\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
@@ -811,6 +818,10 @@ UdfGetInfo (
         *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
@@ -830,7 +841,11 @@ UdfGetInfo (
       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
@@ -840,8 +855,11 @@ UdfGetInfo (
     }\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

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
+  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
@@ -1466,7 +1467,7 @@ InternalFindFile (
         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
@@ -1763,6 +1764,11 @@ FindFile (
   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
@@ -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
+  @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]   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
+  @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
+  IN   UINTN                           CharMax,\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
@@ -1982,10 +2004,22 @@ GetFileNameFromFid (
     return EFI_VOLUME_CORRUPTED;\r
   }\r
 \r
+  FileNameBak = FileName;\r
+\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
@@ -2000,7 +2034,11 @@ GetFileNameFromFid (
     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
@@ -2008,6 +2046,12 @@ GetFileNameFromFid (
 /**\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
@@ -2136,6 +2180,9 @@ ResolveSymlink (
                           Index) << 8;\r
           Index++;\r
         } else {\r
+          if (Index > ARRAY_SIZE (FileName)) {\r
+            return EFI_UNSUPPORTED;\r
+          }\r
           *Char = 0;\r
         }\r
 \r
@@ -2146,7 +2193,11 @@ ResolveSymlink (
         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

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
+  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
@@ -559,9 +560,16 @@ UdfSetPosition (
 /**\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 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
@@ -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_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
@@ -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
+  @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]   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
+  @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
+  IN   UINTN                           CharMax,\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