File ovmf-bsc1130267-overflow-in-partition-and-udf.p... of Package ovmf

Overview Repositories Revisions Requests Users Attributes Meta

File ovmf-bsc1130267-overflow-in-partition-and-udf.patch of Package ovmf (Revision 12f4584862138436fc02d63a13d57176)

Currently displaying revision 12f4584862138436fc02d63a13d57176 , Show latest

From 7e553e84a832140c54b9fe3fecff0ec857d215c3 Mon Sep 17 00:00:00 2001
From: Hao Wu <hao.a.wu@intel.com>
Date: Tue, 12 Dec 2017 16:30:18 +0800
Subject: [PATCH 1/5] 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>
(cherry picked from commit b9ae1705adfdd43668027a25a2b03c2e81960219)
---
 MdeModulePkg/Universal/Disk/UdfDxe/File.c     | 30 +++++++--
 .../Disk/UdfDxe/FileSystemOperations.c        | 65 +++++++++++++++++--
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h      | 30 ++++++++-
 3 files changed, 110 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 6f07bf2066dc..bd723d0257be 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,
@@ -444,7 +445,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;
@@ -456,7 +457,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;
       }
@@ -718,6 +719,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
@@ -794,6 +801,10 @@ UdfGetInfo (
         *String = *(UINT8 *)(OstaCompressed + Index) << 8;
         Index++;
       } else {
+        if (Index > ARRAY_SIZE (VolumeLabel)) {
+          return EFI_VOLUME_CORRUPTED;
+        }
+
         *String = 0;
       }
 
@@ -813,7 +824,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);
@@ -823,8 +838,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 e048d95d3120..3f6ebddae4a8 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
@@ -1412,7 +1413,7 @@ InternalFindFile (
         break;
       }
     } else {
-      Status = GetFileNameFromFid (FileIdentifierDesc, FoundFileName);
+      Status = GetFileNameFromFid (FileIdentifierDesc, ARRAY_SIZE (FoundFileName), FoundFileName);
       if (EFI_ERROR (Status)) {
         break;
       }
@@ -1703,6 +1704,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++;
     }
 
@@ -1880,22 +1886,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 *)(
@@ -1908,10 +1930,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;
@@ -1926,7 +1960,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;
 }
@@ -1934,6 +1972,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.
@@ -2052,6 +2096,9 @@ ResolveSymlink (
                           Index) << 8;
           Index++;
         } else {
+          if (Index > ARRAY_SIZE (FileName)) {
+            return EFI_UNSUPPORTED;
+          }
           *Char = 0;
         }
 
@@ -2062,7 +2109,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 d441539d162d..9b82441e720b 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.
-- 
2.20.1

From 10ed833c05a1077fe472e790148adcea1e25a4e9 Mon Sep 17 00:00:00 2001
From: Hao Wu <hao.a.wu@intel.com>
Date: Wed, 13 Dec 2017 10:23:41 +0800
Subject: [PATCH 2/5] MdeModulePkg/UdfDxe: Add boundary check the read of
 FE/EFE

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

Within ReadFile():

Add checks to ensure that when getting the raw data or the Allocation
Descriptors' data from a FE/EFE, it will not consume data beyond the
size of a FE/EFE.

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>
(cherry picked from commit 5c0748f43f4e1cc15fdd0be64a764eacd7df92f6)
---
 .../Disk/UdfDxe/FileSystemOperations.c        | 54 +++++++++++++++++--
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 3f6ebddae4a8..bb8fd4fb68bd 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -504,15 +504,27 @@ DuplicateFe (
 
   NOTE: The FE/EFE can be thought it was an inode.
 
+  @attention This is boundary function that may receive untrusted input.
+  @attention The input is from FileSystem.
+
+  The (Extended) File Entry is external input, so this routine will do basic
+  validation for (Extended) File Entry and report status.
+
   @param[in]  FileEntryData       (Extended) File Entry pointer.
+  @param[in]  FileEntrySize       Size of the (Extended) File Entry specified
+                                  by FileEntryData.
   @param[out] Data                Buffer contains the raw data of a given
                                   (Extended) File Entry.
   @param[out] Length              Length of the data in Buffer.
 
+  @retval EFI_SUCCESS             Raw data and size of the FE/EFE was read.
+  @retval EFI_VOLUME_CORRUPTED    The file system structures are corrupted.
+
 **/
-VOID
+EFI_STATUS
 GetFileEntryData (
   IN   VOID    *FileEntryData,
+  IN   UINTN   FileEntrySize,
   OUT  VOID    **Data,
   OUT  UINT64  *Length
   )
@@ -536,20 +548,40 @@ GetFileEntryData (
     *Data    = (VOID *)((UINT8 *)FileEntry->Data +
                         FileEntry->LengthOfExtendedAttributes);
   }
+
+  if ((*Length > FileEntrySize) ||
+      ((UINTN)FileEntryData > (UINTN)(*Data)) ||
+      ((UINTN)(*Data) - (UINTN)FileEntryData > FileEntrySize - *Length)) {
+    return EFI_VOLUME_CORRUPTED;
+  }
+  return EFI_SUCCESS;
 }
 
 /**
   Get Allocation Descriptors' data information from a given FE/EFE.
 
+  @attention This is boundary function that may receive untrusted input.
+  @attention The input is from FileSystem.
+
+  The (Extended) File Entry is external input, so this routine will do basic
+  validation for (Extended) File Entry and report status.
+
   @param[in]  FileEntryData       (Extended) File Entry pointer.
+  @param[in]  FileEntrySize       Size of the (Extended) File Entry specified
+                                  by FileEntryData.
   @param[out] AdsData             Buffer contains the Allocation Descriptors'
                                   data from a given FE/EFE.
   @param[out] Length              Length of the data in AdsData.
 
+  @retval EFI_SUCCESS             The data and size of Allocation Descriptors
+                                  were read from the FE/EFE.
+  @retval EFI_VOLUME_CORRUPTED    The file system structures are corrupted.
+
 **/
-VOID
+EFI_STATUS
 GetAdsInformation (
   IN   VOID    *FileEntryData,
+  IN   UINTN   FileEntrySize,
   OUT  VOID    **AdsData,
   OUT  UINT64  *Length
   )
@@ -573,6 +605,13 @@ GetAdsInformation (
     *AdsData = (VOID *)((UINT8 *)FileEntry->Data +
                         FileEntry->LengthOfExtendedAttributes);
   }
+
+  if ((*Length > FileEntrySize) ||
+      ((UINTN)FileEntryData > (UINTN)(*AdsData)) ||
+      ((UINTN)(*AdsData) - (UINTN)FileEntryData > FileEntrySize - *Length)) {
+    return EFI_VOLUME_CORRUPTED;
+  }
+  return EFI_SUCCESS;
 }
 
 /**
@@ -1066,7 +1105,10 @@ ReadFile (
     //
     // There are no extents for this FE/EFE. All data is inline.
     //
-    GetFileEntryData (FileEntryData, &Data, &Length);
+    Status = GetFileEntryData (FileEntryData, Volume->FileEntrySize, &Data, &Length);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
 
     if (ReadFileInfo->Flags == ReadFileGetFileSize) {
       ReadFileInfo->ReadLength = Length;
@@ -1110,7 +1152,11 @@ ReadFile (
     // This FE/EFE contains a run of Allocation Descriptors. Get data + size
     // for start reading them out.
     //
-    GetAdsInformation (FileEntryData, &Data, &Length);
+    Status = GetAdsInformation (FileEntryData, Volume->FileEntrySize, &Data, &Length);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
     AdOffset = 0;
 
     for (;;) {
-- 
2.20.1

From e8dcd9d262bcbb755573b44522aa558d2879f1c1 Mon Sep 17 00:00:00 2001
From: Hao Wu <hao.a.wu@intel.com>
Date: Wed, 13 Dec 2017 14:39:07 +0800
Subject: [PATCH 3/5] MdeModulePkg/UdfDxe: Add boundary check for
 ComponentIdentifier decode

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

Within ResolveSymlink():

The boundary check will validate the 'LengthofComponentIdentifier' field
of a Path Component matches the data within the relating (Extended) File
Entry.

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>
(cherry picked from commit 89f75aa04a97293a8ed9db2a90851a5053730cf5)
---
 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index bb8fd4fb68bd..ca9d828eb4c7 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2135,6 +2135,10 @@ ResolveSymlink (
         return EFI_VOLUME_CORRUPTED;
       }
 
+      if ((UINTN)PathComp->ComponentIdentifier + PathCompLength > (UINTN)EndData) {
+        return EFI_VOLUME_CORRUPTED;
+      }
+
       Char = FileName;
       for (Index = 1; Index < PathCompLength; Index++) {
         if (CompressionId == 16) {
-- 
2.20.1

From c7e4427320be645324915f51e8c8e69d9cc3bb89 Mon Sep 17 00:00:00 2001
From: Hao Wu <hao.a.wu@intel.com>
Date: Wed, 13 Dec 2017 16:28:33 +0800
Subject: [PATCH 4/5] MdeModulePkg/UdfDxe: Add boundary check for getting
 volume (free) size

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

Within GetVolumeSize():

The boundary check will validate the 'NumberOfPartitions' field of a
Logical Volume Integrity Descriptor matches the data within the relating
Logical Volume Descriptor.

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>
(cherry picked from commit 3b30351b75d70ea65701ac999875fbb81a89a5ca)
---
 .../Disk/UdfDxe/FileSystemOperations.c          | 17 ++++++++++++++++-
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h        |  7 +++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index ca9d828eb4c7..223608ada44a 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2449,6 +2449,13 @@ SetFileInfo (
 /**
   Get volume and free space size information of an UDF volume.
 
+  @attention This is boundary function that may receive untrusted input.
+  @attention The input is from FileSystem.
+
+  The Logical Volume Descriptor and the Logical Volume Integrity Descriptor are
+  external inputs, so this routine will do basic validation for both descriptors
+  and report status.
+
   @param[in]   BlockIo        BlockIo interface.
   @param[in]   DiskIo         DiskIo interface.
   @param[in]   Volume         UDF volume information structure.
@@ -2487,7 +2494,8 @@ GetVolumeSize (
 
   ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
 
-  if (ExtentAd->ExtentLength == 0) {
+  if ((ExtentAd->ExtentLength == 0) ||
+      (ExtentAd->ExtentLength < sizeof (UDF_LOGICAL_VOLUME_INTEGRITY))) {
     return EFI_VOLUME_CORRUPTED;
   }
 
@@ -2527,6 +2535,13 @@ GetVolumeSize (
     goto Out_Free;
   }
 
+  if ((LogicalVolInt->NumberOfPartitions > MAX_UINT32 / sizeof (UINT32) / 2) ||
+      (LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2 >
+       ExtentAd->ExtentLength - sizeof (UDF_LOGICAL_VOLUME_INTEGRITY))) {
+    Status = EFI_VOLUME_CORRUPTED;
+    goto Out_Free;
+  }
+
   *VolumeSize = 0;
   *FreeSpaceSize = 0;
 
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index 9b82441e720b..b054c623e5a5 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -903,6 +903,13 @@ SetFileInfo (
 /**
   Get volume and free space size information of an UDF volume.
 
+  @attention This is boundary function that may receive untrusted input.
+  @attention The input is from FileSystem.
+
+  The Logical Volume Descriptor and the Logical Volume Integrity Descriptor are
+  external inputs, so this routine will do basic validation for both descriptors
+  and report status.
+
   @param[in]   BlockIo        BlockIo interface.
   @param[in]   DiskIo         DiskIo interface.
   @param[in]   Volume         UDF volume information structure.
-- 
2.20.1

From 65b766669d3ca2873d0c920f445d1c90e5bc7443 Mon Sep 17 00:00:00 2001
From: Hao Wu <hao.a.wu@intel.com>
Date: Fri, 15 Dec 2017 11:22:16 +0800
Subject: [PATCH 5/5] MdeModulePkg/PartitionDxe: Add check for underlying
 device block size

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

Within FindAnchorVolumeDescriptorPointer():

Add a check for the underlying device block size to ensure it is greater
than the size of an Anchor Volume Descriptor Pointer.

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>
(cherry picked from commit 4df8f5bfa28b8b881e506437e8f08d92c1a00370)
---
 .../Universal/Disk/PartitionDxe/Udf.c         | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index 7eee74895872..75aca7921be4 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -1,7 +1,16 @@
 /** @file
   Scan for an UDF file system on a formatted media.
 
+  Caution: This file requires additional review when modified.
+  This driver will have external input - CD/DVD media.
+  This external input must be validated carefully to avoid security issue like
+  buffer overflow, integer overflow.
+
+  FindUdfFileSystem() routine will consume the media properties and do basic
+  validation.
+
   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
@@ -100,6 +109,20 @@ FindAnchorVolumeDescriptorPointer (
   *LastRecordedBlock = EndLBA;
   AvdpsCount = 0;
 
+  //
+  // Check if the block size of the underlying media can hold the data of an
+  // Anchor Volume Descriptor Pointer
+  //
+  if (BlockSize < sizeof (UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Media block size 0x%x unable to hold an AVDP.\n",
+      __FUNCTION__,
+      BlockSize
+      ));
+    return EFI_UNSUPPORTED;
+  }
+
   //
   // Find AVDP at block 256
   //
@@ -597,6 +620,12 @@ Out_Free:
 /**
   Find a supported UDF file system in block device.
 
+  @attention This is boundary function that may receive untrusted input.
+  @attention The input is from Partition.
+
+  The CD/DVD media is the external input, so this routine will do basic
+  validation for the media.
+
   @param[in]  BlockIo             BlockIo interface.
   @param[in]  DiskIo              DiskIo interface.
   @param[out] StartingLBA         UDF file system starting LBA.
-- 
2.20.1

Places

File ovmf-bsc1130267-overflow-in-partition-and-udf.patch of Package ovmf (Revision 12f4584862138436fc02d63a13d57176)

Places