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.43; helo=mga05.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 08438211799D1 for ; Tue, 16 Oct 2018 00:23:48 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Oct 2018 00:23:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,387,1534834800"; d="scan'208";a="97812131" Received: from shwdeopenpsi014.ccr.corp.intel.com ([10.239.9.9]) by fmsmga004.fm.intel.com with ESMTP; 16 Oct 2018 00:23:46 -0700 From: Hao Wu To: edk2-devel@lists.01.org Cc: Hao Wu , Paulo Alcantara , Ruiyu Ni , Jiewen Yao , Star Zeng Date: Tue, 16 Oct 2018 15:23:33 +0800 Message-Id: <20181016072340.22068-4-hao.a.wu@intel.com> X-Mailer: git-send-email 2.12.0.windows.1 In-Reply-To: <20181016072340.22068-1-hao.a.wu@intel.com> References: <20181016072340.22068-1-hao.a.wu@intel.com> Subject: [PATCH v1 03/10] MdeModulePkg/UdfDxe: Add boundary check the read of FE/EFE 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, 16 Oct 2018 07:23:48 -0000 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: Paulo Alcantara Cc: Ruiyu Ni Cc: Jiewen Yao Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- MdeModulePkg/Universal/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 5fb88c2ff3..d758b798f1 100644 --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c @@ -503,15 +503,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 ) @@ -535,20 +547,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 ) @@ -572,6 +604,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; } /** @@ -1065,7 +1104,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; @@ -1109,7 +1151,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.12.0.windows.1