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.93; helo=mga11.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 3A9A721959CB2 for ; Tue, 26 Feb 2019 08:04:31 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Feb 2019 08:04:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,416,1544515200"; d="scan'208";a="278001622" Received: from shwdeopenpsi777.ccr.corp.intel.com ([10.239.158.28]) by orsmga004.jf.intel.com with ESMTP; 26 Feb 2019 08:04:29 -0800 From: Jian J Wang To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao , Liming Gao , Hao Wu Date: Wed, 27 Feb 2019 00:04:24 +0800 Message-Id: <20190226160425.4816-3-jian.j.wang@intel.com> X-Mailer: git-send-email 2.17.1.windows.2 In-Reply-To: <20190226160425.4816-1-jian.j.wang@intel.com> References: <20190226160425.4816-1-jian.j.wang@intel.com> Subject: [PATCH 2/3] MdeModulePkg/DxeCore: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630] X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Feb 2019 16:04:31 -0000 From: Star Zeng REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864 To follow PI spec, ensure FfsFileHeader 8 bytes aligned. For the integrity of FV(especially non-MemoryMapped FV) layout, let CachedFv point to FV beginning, but not (FV + FV header). And current code only handles (FwVolHeader->ExtHeaderOffset != 0) path, update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path. Cc: Jiewen Yao Cc: Liming Gao Cc: Jian J Wang Cc: Hao Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Core/Dxe/FwVol/FwVol.c | 65 +++++++---------------------- 1 file changed, 14 insertions(+), 51 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c index 93ddcc3591..28fce46a95 100644 --- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c @@ -3,7 +3,7 @@ Layers on top of Firmware Block protocol to produce a file abstraction of FV based files. -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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 @@ -329,8 +329,6 @@ FvCheck ( FFS_FILE_LIST_ENTRY *FfsFileEntry; EFI_FFS_FILE_HEADER *FfsHeader; UINT8 *CacheLocation; - UINTN LbaOffset; - UINTN HeaderSize; UINTN Index; EFI_LBA LbaIndex; UINTN Size; @@ -353,11 +351,7 @@ FvCheck ( return Status; } - // - // Size is the size of the FV minus the head. We have already allocated - // the header to check to make sure the volume is valid - // - Size = (UINTN)(FwVolHeader->FvLength - FwVolHeader->HeaderLength); + Size = (UINTN) FwVolHeader->FvLength; if ((FvbAttributes & EFI_FVB2_MEMORY_MAPPED) != 0) { FvDevice->IsMemoryMapped = TRUE; @@ -369,7 +363,7 @@ FvCheck ( // // Don't cache memory mapped FV really. // - FvDevice->CachedFv = (UINT8 *) (UINTN) (PhysicalAddress + FwVolHeader->HeaderLength); + FvDevice->CachedFv = (UINT8 *) (UINTN) PhysicalAddress; } else { FvDevice->IsMemoryMapped = FALSE; FvDevice->CachedFv = AllocatePool (Size); @@ -380,52 +374,27 @@ FvCheck ( } // - // Remember a pointer to the end fo the CachedFv + // Remember a pointer to the end of the CachedFv // FvDevice->EndOfCachedFv = FvDevice->CachedFv + Size; if (!FvDevice->IsMemoryMapped) { // - // Copy FV minus header into memory using the block map we have all ready - // read into memory. + // Copy FV into memory using the block map. // BlockMap = FwVolHeader->BlockMap; CacheLocation = FvDevice->CachedFv; LbaIndex = 0; - LbaOffset = 0; - HeaderSize = FwVolHeader->HeaderLength; while ((BlockMap->NumBlocks != 0) || (BlockMap->Length != 0)) { - Index = 0; - Size = BlockMap->Length; - if (HeaderSize > 0) { - // - // Skip header size - // - for (; Index < BlockMap->NumBlocks && HeaderSize >= BlockMap->Length; Index ++) { - HeaderSize -= BlockMap->Length; - LbaIndex ++; - } - - // - // Check whether FvHeader is crossing the multi block range. - // - if (Index >= BlockMap->NumBlocks) { - BlockMap++; - continue; - } else if (HeaderSize > 0) { - LbaOffset = HeaderSize; - Size = BlockMap->Length - HeaderSize; - HeaderSize = 0; - } - } - // // read the FV data // - for (; Index < BlockMap->NumBlocks; Index ++) { - Status = Fvb->Read (Fvb, + Size = BlockMap->Length; + for (Index = 0; Index < BlockMap->NumBlocks; Index++) { + Status = Fvb->Read ( + Fvb, LbaIndex, - LbaOffset, + 0, &Size, CacheLocation ); @@ -438,13 +407,7 @@ FvCheck ( } LbaIndex++; - CacheLocation += Size; - - // - // After we skip Fv Header always read from start of block - // - LbaOffset = 0; - Size = BlockMap->Length; + CacheLocation += BlockMap->Length; } BlockMap++; @@ -475,12 +438,12 @@ FvCheck ( // // Searching for files starts on an 8 byte aligned boundary after the end of the Extended Header if it exists. // - FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (FvDevice->CachedFv + (FwVolHeader->ExtHeaderOffset - FwVolHeader->HeaderLength)); + FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (FvDevice->CachedFv + FwVolHeader->ExtHeaderOffset); FfsHeader = (EFI_FFS_FILE_HEADER *) ((UINT8 *) FwVolExtHeader + FwVolExtHeader->ExtHeaderSize); - FfsHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsHeader, 8); } else { - FfsHeader = (EFI_FFS_FILE_HEADER *) (FvDevice->CachedFv); + FfsHeader = (EFI_FFS_FILE_HEADER *) (FvDevice->CachedFv + FwVolHeader->HeaderLength); } + FfsHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsHeader, 8); TopFvAddress = FvDevice->EndOfCachedFv; while (((UINTN) FfsHeader >= (UINTN) FvDevice->CachedFv) && ((UINTN) FfsHeader <= (UINTN) ((UINTN) TopFvAddress - sizeof (EFI_FFS_FILE_HEADER)))) { -- 2.17.1.windows.2