public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Hao Wu <hao.a.wu@intel.com>
To: edk2-devel@lists.01.org
Cc: Hao Wu <hao.a.wu@intel.com>, Paulo Alcantara <paulo@paulo.ac>,
	Paulo Alcantara <palcantara@suse.de>,
	Ruiyu Ni <ruiyu.ni@intel.com>, Star Zeng <star.zeng@intel.com>
Subject: [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink()
Date: Fri, 26 Oct 2018 15:54:57 +0800	[thread overview]
Message-ID: <20181026075457.6280-1-hao.a.wu@intel.com> (raw)

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

The commit will add 3 types of checks for function ResolveSymlink():

A. Check for the value of 'Component Type' field within a Path Component

According to the ECMA-167 standard (3rd Edition - June 1997), Section
14.16.1.1, valid values are 1 to 5. All other values will be treated as a
corrupted volume.

B. Check for the content pointed by 'File'

Since content within 'File' is the output data for ResolveSymlink().
Checks is added to ensure the content in 'File' is valid. Otherwise,
possible null pointer dereference issue will occur during the subsequent
usage of the data returned by ResolveSymlink().

C. Check for possible memory double free/use after free case

For codes:

    if (CompareMem ((VOID *)&PreviousFile, (VOID *)Parent,
                    sizeof (UDF_FILE_INFO)) != 0) {
      CleanupFileInformation (&PreviousFile);
    }

    CopyMem ((VOID *)&PreviousFile, (VOID *)File, sizeof (UDF_FILE_INFO));

If the contents in 'PreviousFile' and 'File' are the same, call to
"CleanupFileInformation (&PreviousFile);" will free the buffers in 'File'
as well. This will lead to potential memory double free/use after free
issues.

Cc: Paulo Alcantara <paulo@paulo.ac>
Cc: Paulo Alcantara <palcantara@suse.de>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 30 ++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index b9ebddfe62..a89e5ba9ff 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2145,6 +2145,8 @@ ResolveSymlink (
   UINT8               CompressionId;
   UDF_FILE_INFO       PreviousFile;
 
+  ZeroMem ((VOID *)File, sizeof (UDF_FILE_INFO));
+
   //
   // Symlink files on UDF volumes do not contain so much data other than
   // Path Components which resolves to real filenames, so it's OK to read in
@@ -2257,6 +2259,13 @@ ResolveSymlink (
       }
       FileName[Index] = L'\0';
       break;
+    default:
+      //
+      // Accoring to the ECMA-167 standard (3rd Edition - June 1997), Section
+      // 14.16.1.1, all other values are reserved.
+      //
+      Status = EFI_VOLUME_CORRUPTED;
+      goto Error_Find_File;
     }
 
     //
@@ -2281,8 +2290,18 @@ ResolveSymlink (
       break;
     }
 
-    if (CompareMem ((VOID *)&PreviousFile, (VOID *)Parent,
-                    sizeof (UDF_FILE_INFO)) != 0) {
+    //
+    // Check the content in the file info pointed by File.
+    //
+    if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
+      Status = EFI_VOLUME_CORRUPTED;
+      goto Error_Find_File;
+    }
+
+    if ((CompareMem ((VOID *)&PreviousFile, (VOID *)Parent,
+                    sizeof (UDF_FILE_INFO)) != 0) &&
+        (CompareMem ((VOID *)&PreviousFile, (VOID *)File,
+                    sizeof (UDF_FILE_INFO)) != 0)) {
       CleanupFileInformation (&PreviousFile);
     }
 
@@ -2294,6 +2313,13 @@ ResolveSymlink (
   //
   FreePool (ReadFileInfo.FileData);
 
+  //
+  // Check the content in the resolved file info.
+  //
+  if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
+    return EFI_VOLUME_CORRUPTED;
+  }
+
   return EFI_SUCCESS;
 
 Error_Find_File:
-- 
2.12.0.windows.1



             reply	other threads:[~2018-10-26  7:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-26  7:54 Hao Wu [this message]
2018-10-26 13:26 ` [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink() Paulo Alcantara
2018-10-26 14:14 ` Zeng, Star
2018-10-27 12:28   ` Wu, Hao A
2018-10-29 11:09 ` Leif Lindholm
2018-10-30  1:01   ` Wu, Hao A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181026075457.6280-1-hao.a.wu@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox