From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.151; helo=mga17.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EEA6C22361E44 for ; Wed, 7 Feb 2018 20:39:12 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Feb 2018 20:44:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,476,1511856000"; d="scan'208";a="16858352" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.19]) by orsmga006.jf.intel.com with ESMTP; 07 Feb 2018 20:44:55 -0800 From: Ruiyu Ni To: edk2-devel@lists.01.org Cc: Jaben Carsey Date: Thu, 8 Feb 2018 12:44:54 +0800 Message-Id: <20180208044454.91652-1-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 Subject: [PATCH] ShellPkg/hexedit: Fix a read-after-free bug X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Feb 2018 04:39:13 -0000 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 Cc: Jaben Carsey --- .../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.
+ Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
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.
+ Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
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