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>

3 files changed, 110 insertions, 15 deletions

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

 

   Copyright (C) 2014-2017 Paulo Alcantara <pcacjr@zytor.com>

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

 

   This program and the accompanying materials are licensed and made available

   under the terms and conditions of the BSD License which accompanies this

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

   }

 

-  StrCpyS (NewPrivFileData->FileName, UDF_PATH_LENGTH, FileName);

+  StrCpyS (NewPrivFileData->FileName, UDF_FILENAME_LENGTH, FileName);

 

   Status = GetFileSize (

     PrivFsData->BlockIo,

@@ -457,7 +458,7 @@ UdfRead (
       FreePool ((VOID *)NewFileEntryData);

       NewFileEntryData = FoundFile.FileEntry;

 

-      Status = GetFileNameFromFid (NewFileIdentifierDesc, FileName);

+      Status = GetFileNameFromFid (NewFileIdentifierDesc, ARRAY_SIZE (FileName), FileName);

       if (EFI_ERROR (Status)) {

         FreePool ((VOID *)FoundFile.FileIdentifierDesc);

         goto Error_Get_FileName;

@@ -469,7 +470,7 @@ UdfRead (
       FoundFile.FileIdentifierDesc  = NewFileIdentifierDesc;

       FoundFile.FileEntry           = NewFileEntryData;

 

-      Status = GetFileNameFromFid (FoundFile.FileIdentifierDesc, FileName);

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

       if (EFI_ERROR (Status)) {

         goto Error_Get_FileName;

       }

@@ -735,6 +736,12 @@ UdfSetPosition (
 /**

   Get information about a file.

 

+  @attention This is boundary function that may receive untrusted input.

+  @attention The input is from FileSystem.

+

+  The File Set Descriptor is external input, so this routine will do basic

+  validation for File Set Descriptor and report status.

+

   @param  This            Protocol instance pointer.

   @param  InformationType Type of information to return in Buffer.

   @param  BufferSize      On input size of buffer, on output amount of data in

@@ -811,6 +818,10 @@ UdfGetInfo (
         *String = *(UINT8 *)(OstaCompressed + Index) << 8;

         Index++;

       } else {

+        if (Index > ARRAY_SIZE (VolumeLabel)) {

+          return EFI_VOLUME_CORRUPTED;

+        }

+

         *String = 0;

       }

 

@@ -830,7 +841,11 @@ UdfGetInfo (
       String++;

     }

 

-    *String = L'\0';

+    Index = ((UINTN)String - (UINTN)VolumeLabel) / sizeof (CHAR16);

+    if (Index > ARRAY_SIZE (VolumeLabel) - 1) {

+      Index = ARRAY_SIZE (VolumeLabel) - 1;

+    }

+    VolumeLabel[Index] = L'\0';

 

     FileSystemInfoLength = StrSize (VolumeLabel) +

                            sizeof (EFI_FILE_SYSTEM_INFO);

@@ -840,8 +855,11 @@ UdfGetInfo (
     }

 

     FileSystemInfo = (EFI_FILE_SYSTEM_INFO *)Buffer;

-    StrCpyS (FileSystemInfo->VolumeLabel, ARRAY_SIZE (VolumeLabel),

-             VolumeLabel);

+    StrCpyS (

+      FileSystemInfo->VolumeLabel,

+      (*BufferSize - OFFSET_OF (EFI_FILE_SYSTEM_INFO, VolumeLabel)) / sizeof (CHAR16),

+      VolumeLabel

+      );

     Status = GetVolumeSize (

       PrivFsData->BlockIo,

       PrivFsData->DiskIo,

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 9048a18..b3ac673 100644
--- 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.

 

   Copyright (C) 2014-2017 Paulo Alcantara <pcacjr@zytor.com>

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

 

   This program and the accompanying materials are licensed and made available

   under the terms and conditions of the BSD License which accompanies this

@@ -1466,7 +1467,7 @@ InternalFindFile (
         break;

       }

     } else {

-      Status = GetFileNameFromFid (FileIdentifierDesc, FoundFileName);

+      Status = GetFileNameFromFid (FileIdentifierDesc, ARRAY_SIZE (FoundFileName), FoundFileName);

       if (EFI_ERROR (Status)) {

         break;

       }

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

     FileNamePointer = FileName;

     while (*FilePath != L'\0' && *FilePath != L'\\') {

+      if ((((UINTN)FileNamePointer - (UINTN)FileName) / sizeof (CHAR16)) >=

+          (ARRAY_SIZE (FileName) - 1)) {

+        return EFI_NOT_FOUND;

+      }

+

       *FileNamePointer++ = *FilePath++;

     }

 

@@ -1954,22 +1960,38 @@ ReadDirectoryEntry (
   Get a filename (encoded in OSTA-compressed format) from a File Identifier

   Descriptor on an UDF volume.

 

+  @attention This is boundary function that may receive untrusted input.

+  @attention The input is from FileSystem.

+

+  The File Identifier Descriptor is external input, so this routine will do

+  basic validation for File Identifier Descriptor and report status.

+

   @param[in]   FileIdentifierDesc  File Identifier Descriptor pointer.

+  @param[in]   CharMax             The maximum number of FileName Unicode char,

+                                   including terminating null char.

   @param[out]  FileName            Decoded filename.

 

   @retval EFI_SUCCESS           Filename decoded and read.

   @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.

+  @retval EFI_BUFFER_TOO_SMALL  The string buffer FileName cannot hold the

+                                decoded filename.

 **/

 EFI_STATUS

 GetFileNameFromFid (

   IN   UDF_FILE_IDENTIFIER_DESCRIPTOR  *FileIdentifierDesc,

+  IN   UINTN                           CharMax,

   OUT  CHAR16                          *FileName

   )

 {

-  UINT8 *OstaCompressed;

-  UINT8 CompressionId;

-  UINT8 Length;

-  UINTN Index;

+  UINT8  *OstaCompressed;

+  UINT8  CompressionId;

+  UINT8  Length;

+  UINTN  Index;

+  CHAR16 *FileNameBak;

+

+  if (CharMax == 0) {

+    return EFI_BUFFER_TOO_SMALL;

+  }

 

   OstaCompressed =

     (UINT8 *)(

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

   }

 

+  FileNameBak = FileName;

+

   //

   // Decode filename.

   //

   Length = FileIdentifierDesc->LengthOfFileIdentifier;

+  if (CompressionId == 16) {

+    if (((UINTN)Length >> 1) > CharMax) {

+      return EFI_BUFFER_TOO_SMALL;

+    }

+  } else {

+    if ((Length != 0) && ((UINTN)Length - 1 > CharMax)) {

+      return EFI_BUFFER_TOO_SMALL;

+    }

+  }

+

   for (Index = 1; Index < Length; Index++) {

     if (CompressionId == 16) {

       *FileName = OstaCompressed[Index++] << 8;

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

   }

 

-  *FileName = L'\0';

+  Index = ((UINTN)FileName - (UINTN)FileNameBak) / sizeof (CHAR16);

+  if (Index > CharMax - 1) {

+    Index = CharMax - 1;

+  }

+  FileNameBak[Index] = L'\0';

 

   return EFI_SUCCESS;

 }

@@ -2008,6 +2046,12 @@ GetFileNameFromFid (
 /**

   Resolve a symlink file on an UDF volume.

 

+  @attention This is boundary function that may receive untrusted input.

+  @attention The input is from FileSystem.

+

+  The Path Component is external input, so this routine will do basic

+  validation for Path Component and report status.

+

   @param[in]   BlockIo        BlockIo interface.

   @param[in]   DiskIo         DiskIo interface.

   @param[in]   Volume         UDF volume information structure.

@@ -2136,6 +2180,9 @@ ResolveSymlink (
                           Index) << 8;

           Index++;

         } else {

+          if (Index > ARRAY_SIZE (FileName)) {

+            return EFI_UNSUPPORTED;

+          }

           *Char = 0;

         }

 

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

       }

 

-      *Char = L'\0';

+      Index = ((UINTN)Char - (UINTN)FileName) / sizeof (CHAR16);

+      if (Index > ARRAY_SIZE (FileName) - 1) {

+        Index = ARRAY_SIZE (FileName) - 1;

+      }

+      FileName[Index] = L'\0';

       break;

     }

 

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index d441539..9b82441 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -2,6 +2,7 @@
   UDF/ECMA-167 file system driver.

 

   Copyright (C) 2014-2017 Paulo Alcantara <pcacjr@zytor.com>

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

 

   This program and the accompanying materials are licensed and made available

   under the terms and conditions of the BSD License which accompanies this

@@ -559,9 +560,16 @@ UdfSetPosition (
 /**

   Get information about a file.

 

+  @attention This is boundary function that may receive untrusted input.

+  @attention The input is from FileSystem.

+

+  The File Set Descriptor is external input, so this routine will do basic

+  validation for File Set Descriptor and report status.

+

   @param  This            Protocol instance pointer.

   @param  InformationType Type of information to return in Buffer.

-  @param  BufferSize      On input size of buffer, on output amount of data in buffer.

+  @param  BufferSize      On input size of buffer, on output amount of data in

+                          buffer.

   @param  Buffer          The buffer to return data.

 

   @retval EFI_SUCCESS          Data was returned.

@@ -571,7 +579,8 @@ UdfSetPosition (
   @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.

   @retval EFI_WRITE_PROTECTED  The device is write protected.

   @retval EFI_ACCESS_DENIED    The file was open for read only.

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

+  @retval EFI_BUFFER_TOO_SMALL Buffer was too small; required size returned in

+                               BufferSize.

 

 **/

 EFI_STATUS

@@ -769,21 +778,38 @@ ReadDirectoryEntry (
   Get a filename (encoded in OSTA-compressed format) from a File Identifier

   Descriptor on an UDF volume.

 

+  @attention This is boundary function that may receive untrusted input.

+  @attention The input is from FileSystem.

+

+  The File Identifier Descriptor is external input, so this routine will do

+  basic validation for File Identifier Descriptor and report status.

+

   @param[in]   FileIdentifierDesc  File Identifier Descriptor pointer.

+  @param[in]   CharMax             The maximum number of FileName Unicode char,

+                                   including terminating null char.

   @param[out]  FileName            Decoded filename.

 

   @retval EFI_SUCCESS           Filename decoded and read.

   @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.

+  @retval EFI_BUFFER_TOO_SMALL  The string buffer FileName cannot hold the

+                                decoded filename.

 **/

 EFI_STATUS

 GetFileNameFromFid (

   IN   UDF_FILE_IDENTIFIER_DESCRIPTOR  *FileIdentifierDesc,

+  IN   UINTN                           CharMax,

   OUT  CHAR16                          *FileName

   );

 

 /**

   Resolve a symlink file on an UDF volume.

 

+  @attention This is boundary function that may receive untrusted input.

+  @attention The input is from FileSystem.

+

+  The Path Component is external input, so this routine will do basic

+  validation for Path Component and report status.

+

   @param[in]   BlockIo        BlockIo interface.

   @param[in]   DiskIo         DiskIo interface.

   @param[in]   Volume         UDF volume information structure.