public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ruiyu Ni <ruiyu.ni@intel.com>
To: edk2-devel@lists.01.org
Cc: Jaben Carsey <jaben.carsey@intel.com>
Subject: [PATCH] ShellPkg/hexedit: Fix a read-after-free bug
Date: Thu,  8 Feb 2018 12:44:54 +0800	[thread overview]
Message-ID: <20180208044454.91652-1-ruiyu.ni@intel.com> (raw)

HDiskImageSetDiskNameOffsetSize() and HFileImageSetFileName()
may be called using the current disk name or file name.
When this happens, today's implementation firstly frees the memory
and then accesses the just-freed memory.
The patch fixes this issue by doing nothing when the disk or file
name is the current one.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
---
 .../UefiShellDebug1CommandsLib/HexEdit/DiskImage.c | 22 +++++++++------------
 .../UefiShellDebug1CommandsLib/HexEdit/FileImage.c | 23 +++++++++-------------
 2 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
index 846b102975..8deb643f07 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
@@ -1,7 +1,7 @@
 /** @file
   Functions to deal with Disk buffer.
 
-  Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved. <BR>
+  Copyright (c) 2005 - 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 distribution.  The full text of the license may be found at
@@ -120,27 +120,23 @@ HDiskImageSetDiskNameOffsetSize (
   IN UINTN    Size
   )
 {
-  UINTN Len;
-  UINTN Index;
+  if (Str == HDiskImage.Name) {
+    //
+    // This function might be called using HDiskImage.FileName as Str.
+    // Directly return without updating HDiskImage.FileName.
+    //
+    return EFI_SUCCESS;
+  }
 
   //
   // free the old file name
   //
   SHELL_FREE_NON_NULL (HDiskImage.Name);
-
-  Len             = StrLen (Str);
-
-  HDiskImage.Name = AllocateZeroPool (2 * (Len + 1));
+  HDiskImage.Name = AllocateCopyPool (StrSize (Str), Str);
   if (HDiskImage.Name == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  for (Index = 0; Index < Len; Index++) {
-    HDiskImage.Name[Index] = Str[Index];
-  }
-
-  HDiskImage.Name[Len]  = L'\0';
-
   HDiskImage.Offset     = Offset;
   HDiskImage.Size       = Size;
 
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/FileImage.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/FileImage.c
index 2517a57f59..d9fd72cdd2 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/FileImage.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/FileImage.c
@@ -1,7 +1,7 @@
 /** @file
   Functions to deal with file buffer.
 
-  Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved. <BR>
+  Copyright (c) 2005 - 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 distribution.  The full text of the license may be found at
@@ -110,27 +110,22 @@ HFileImageSetFileName (
   IN CONST CHAR16 *Str
   )
 {
-  UINTN Size;
-  UINTN Index;
-
+  if (Str == HFileImage.FileName) {
+    //
+    // This function might be called using HFileImage.FileName as Str.
+    // Directly return without updating HFileImage.FileName.
+    //
+    return EFI_SUCCESS;
+  }
   //
   // free the old file name
   //
   SHELL_FREE_NON_NULL (HFileImage.FileName);
-
-  Size                = StrLen (Str);
-
-  HFileImage.FileName = AllocateZeroPool (2 * (Size + 1));
+  HFileImage.FileName = AllocateCopyPool (StrSize (Str), Str);
   if (HFileImage.FileName == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  for (Index = 0; Index < Size; Index++) {
-    HFileImage.FileName[Index] = Str[Index];
-  }
-
-  HFileImage.FileName[Size] = L'\0';
-
   return EFI_SUCCESS;
 }
 
-- 
2.16.1.windows.1



             reply	other threads:[~2018-02-08  4:39 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-08  4:44 Ruiyu Ni [this message]
2018-02-08 15:22 ` [PATCH] ShellPkg/hexedit: Fix a read-after-free bug Carsey, Jaben

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=20180208044454.91652-1-ruiyu.ni@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