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 1D2C021167478 for ; Fri, 26 Oct 2018 00:55:03 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Oct 2018 00:55:03 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,427,1534834800"; d="scan'208";a="95180420" Received: from shwdeopenpsi014.ccr.corp.intel.com ([10.239.9.9]) by orsmga003.jf.intel.com with ESMTP; 26 Oct 2018 00:55:01 -0700 From: Hao Wu To: edk2-devel@lists.01.org Cc: Hao Wu , Paulo Alcantara , Paulo Alcantara , Ruiyu Ni , Star Zeng Date: Fri, 26 Oct 2018 15:54:57 +0800 Message-Id: <20181026075457.6280-1-hao.a.wu@intel.com> X-Mailer: git-send-email 2.12.0.windows.1 Subject: [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink() 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: Fri, 26 Oct 2018 07:55:04 -0000 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 Cc: Paulo Alcantara Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- .../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