From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DDCBE1A1F05 for ; Wed, 12 Oct 2016 05:21:37 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 12 Oct 2016 05:21:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,482,1473145200"; d="scan'208";a="1063788769" Received: from shwdeopenpsi014.ccr.corp.intel.com ([10.239.9.34]) by orsmga002.jf.intel.com with ESMTP; 12 Oct 2016 05:21:36 -0700 From: Hao Wu To: edk2-devel@lists.01.org Cc: Hao Wu , Liming Gao , Yonghong Zhu , Eric Dong , Dandan Bi Date: Wed, 12 Oct 2016 20:20:08 +0800 Message-Id: <1476274836-10544-25-git-send-email-hao.a.wu@intel.com> X-Mailer: git-send-email 1.9.5.msysgit.0 In-Reply-To: <1476274836-10544-1-git-send-email-hao.a.wu@intel.com> References: <1476274836-10544-1-git-send-email-hao.a.wu@intel.com> Subject: [PATCH 24/52] BaseTools/VfrCompile: Fix potential access over array bounds X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Oct 2016 12:21:38 -0000 Cc: Liming Gao Cc: Yonghong Zhu Cc: Eric Dong Cc: Dandan Bi Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu --- BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h | 3 + BaseTools/Source/C/VfrCompile/VfrCompiler.cpp | 132 +++++++++++++++++++++--- BaseTools/Source/C/VfrCompile/VfrCompiler.h | 8 +- BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 8 ++ 4 files changed, 135 insertions(+), 16 deletions(-) diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h index 37cac24..f15bff1 100644 --- a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h +++ b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h @@ -30,6 +30,8 @@ * 1989-2000 */ +#include + #define ZZINC {if ( track_columns ) (++_endcol);} #define ZZGETC {ch = input->nextChar(); cl = ZZSHIFT(ch);} @@ -114,6 +116,7 @@ more: state = dfa_base[automaton]; while (ZZNEWSTATE != DfaStates) { state = newstate; + assert(state <= sizeof(dfa)/sizeof(dfa[0])); ZZCOPY; ZZGETC; ZZINC; diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp index 59f4bf3..42c9a5e 100644 --- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp +++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp @@ -69,13 +69,13 @@ CVfrCompiler::OptionInitialization ( SetUtilityName ((CHAR8*) PROGRAM_NAME); mOptions.VfrFileName[0] = '\0'; - mOptions.RecordListFile[0] = '\0'; + mOptions.RecordListFile = NULL; mOptions.CreateRecordListFile = FALSE; mOptions.CreateIfrPkgFile = FALSE; - mOptions.PkgOutputFileName[0] = '\0'; - mOptions.COutputFileName[0] = '\0'; + mOptions.PkgOutputFileName = NULL; + mOptions.COutputFileName = NULL; mOptions.OutputDirectory[0] = '\0'; - mOptions.PreprocessorOutputFileName[0] = '\0'; + mOptions.PreprocessorOutputFileName = NULL; mOptions.VfrBaseFileName[0] = '\0'; mOptions.IncludePaths = NULL; mOptions.SkipCPreprocessor = TRUE; @@ -119,7 +119,12 @@ CVfrCompiler::OptionInitialization ( DebugError (NULL, 0, 1001, "Missing option", "-o missing output directory name"); goto Fail; } - strcpy (mOptions.OutputDirectory, Argv[Index]); + if (strlen (Argv[Index]) > MAX_PATH - 1) { + DebugError (NULL, 0, 1003, "Invalid option value", "Output directory name %s is too long", Argv[Index]); + goto Fail; + } + strncpy (mOptions.OutputDirectory, Argv[Index], MAX_PATH - 1); + mOptions.OutputDirectory[MAX_PATH - 1] = 0; CHAR8 lastChar = mOptions.OutputDirectory[strlen(mOptions.OutputDirectory) - 1]; if ((lastChar != '/') && (lastChar != '\\')) { @@ -176,7 +181,12 @@ CVfrCompiler::OptionInitialization ( DebugError (NULL, 0, 1001, "Missing option", "VFR file name is not specified."); goto Fail; } else { - strcpy (mOptions.VfrFileName, Argv[Index]); + if (strlen (Argv[Index]) > MAX_PATH) { + DebugError (NULL, 0, 1003, "Invalid option value", "VFR file name %s is too long.", Argv[Index]); + goto Fail; + } + strncpy (mOptions.VfrFileName, Argv[Index], MAX_PATH - 1); + mOptions.VfrFileName[MAX_PATH - 1] = 0; } if (SetBaseFileName() != 0) { @@ -200,14 +210,26 @@ Fail: SET_RUN_STATUS (STATUS_DEAD); mOptions.VfrFileName[0] = '\0'; - mOptions.RecordListFile[0] = '\0'; mOptions.CreateRecordListFile = FALSE; mOptions.CreateIfrPkgFile = FALSE; - mOptions.PkgOutputFileName[0] = '\0'; - mOptions.COutputFileName[0] = '\0'; mOptions.OutputDirectory[0] = '\0'; - mOptions.PreprocessorOutputFileName[0] = '\0'; mOptions.VfrBaseFileName[0] = '\0'; + if (mOptions.PkgOutputFileName != NULL) { + free (mOptions.PkgOutputFileName); + mOptions.PkgOutputFileName = NULL; + } + if (mOptions.COutputFileName != NULL) { + free (mOptions.COutputFileName); + mOptions.COutputFileName = NULL; + } + if (mOptions.PreprocessorOutputFileName != NULL) { + free (mOptions.PreprocessorOutputFileName); + mOptions.PreprocessorOutputFileName = NULL; + } + if (mOptions.RecordListFile != NULL) { + free (mOptions.RecordListFile); + mOptions.RecordListFile = NULL; + } if (mOptions.IncludePaths != NULL) { delete mOptions.IncludePaths; mOptions.IncludePaths = NULL; @@ -304,8 +326,14 @@ CVfrCompiler::SetBaseFileName ( return -1; } - strncpy (mOptions.VfrBaseFileName, pFileName, pExt - pFileName); - mOptions.VfrBaseFileName[pExt - pFileName] = '\0'; + *pExt = '\0'; + if (strlen (pFileName) > MAX_PATH - 1) { + *pExt = '.'; + return -1; + } + strncpy (mOptions.VfrBaseFileName, pFileName, MAX_PATH - 1); + mOptions.VfrBaseFileName[MAX_PATH - 1] = '\0'; + *pExt = '.'; return 0; } @@ -315,10 +343,25 @@ CVfrCompiler::SetPkgOutputFileName ( VOID ) { + INTN Length; + if (mOptions.VfrBaseFileName[0] == '\0') { return -1; } + Length = strlen (mOptions.OutputDirectory) + + strlen (mOptions.VfrBaseFileName) + + strlen (VFR_PACKAGE_FILENAME_EXTENSION) + + 1; + if (Length > MAX_PATH) { + return -1; + } + + mOptions.PkgOutputFileName = (CHAR8 *) malloc (Length); + if (mOptions.PkgOutputFileName == NULL) { + return -1; + } + strcpy (mOptions.PkgOutputFileName, mOptions.OutputDirectory); strcat (mOptions.PkgOutputFileName, mOptions.VfrBaseFileName); strcat (mOptions.PkgOutputFileName, VFR_PACKAGE_FILENAME_EXTENSION); @@ -331,10 +374,25 @@ CVfrCompiler::SetCOutputFileName ( VOID ) { + INTN Length; + if (mOptions.VfrBaseFileName[0] == '\0') { return -1; } + Length = strlen (mOptions.OutputDirectory) + + strlen (mOptions.VfrBaseFileName) + + strlen (".c") + + 1; + if (Length > MAX_PATH) { + return -1; + } + + mOptions.COutputFileName = (CHAR8 *) malloc (Length); + if (mOptions.COutputFileName == NULL) { + return -1; + } + strcpy (mOptions.COutputFileName, mOptions.OutputDirectory); strcat (mOptions.COutputFileName, mOptions.VfrBaseFileName); strcat (mOptions.COutputFileName, ".c"); @@ -347,10 +405,25 @@ CVfrCompiler::SetPreprocessorOutputFileName ( VOID ) { + INTN Length; + if (mOptions.VfrBaseFileName[0] == '\0') { return -1; } + Length = strlen (mOptions.OutputDirectory) + + strlen (mOptions.VfrBaseFileName) + + strlen (VFR_PREPROCESS_FILENAME_EXTENSION) + + 1; + if (Length > MAX_PATH) { + return -1; + } + + mOptions.PreprocessorOutputFileName = (CHAR8 *) malloc (Length); + if (mOptions.PreprocessorOutputFileName == NULL) { + return -1; + } + strcpy (mOptions.PreprocessorOutputFileName, mOptions.OutputDirectory); strcat (mOptions.PreprocessorOutputFileName, mOptions.VfrBaseFileName); strcat (mOptions.PreprocessorOutputFileName, VFR_PREPROCESS_FILENAME_EXTENSION); @@ -363,10 +436,25 @@ CVfrCompiler::SetRecordListFileName ( VOID ) { + INTN Length; + if (mOptions.VfrBaseFileName[0] == '\0') { return -1; } + Length = strlen (mOptions.OutputDirectory) + + strlen (mOptions.VfrBaseFileName) + + strlen (VFR_RECORDLIST_FILENAME_EXTENSION) + + 1; + if (Length > MAX_PATH) { + return -1; + } + + mOptions.RecordListFile = (CHAR8 *) malloc (Length); + if (mOptions.RecordListFile == NULL) { + return -1; + } + strcpy (mOptions.RecordListFile, mOptions.OutputDirectory); strcat (mOptions.RecordListFile, mOptions.VfrBaseFileName); strcat (mOptions.RecordListFile, VFR_RECORDLIST_FILENAME_EXTENSION); @@ -397,6 +485,26 @@ CVfrCompiler::~CVfrCompiler ( VOID ) { + if (mOptions.PkgOutputFileName != NULL) { + free (mOptions.PkgOutputFileName); + mOptions.PkgOutputFileName = NULL; + } + + if (mOptions.COutputFileName != NULL) { + free (mOptions.COutputFileName); + mOptions.COutputFileName = NULL; + } + + if (mOptions.PreprocessorOutputFileName != NULL) { + free (mOptions.PreprocessorOutputFileName); + mOptions.PreprocessorOutputFileName = NULL; + } + + if (mOptions.RecordListFile != NULL) { + free (mOptions.RecordListFile); + mOptions.RecordListFile = NULL; + } + if (mOptions.IncludePaths != NULL) { delete mOptions.IncludePaths; mOptions.IncludePaths = NULL; diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.h b/BaseTools/Source/C/VfrCompile/VfrCompiler.h index 7dd9dd0..7200cea 100644 --- a/BaseTools/Source/C/VfrCompile/VfrCompiler.h +++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.h @@ -42,13 +42,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. typedef struct { CHAR8 VfrFileName[MAX_PATH]; - CHAR8 RecordListFile[MAX_PATH]; - CHAR8 PkgOutputFileName[MAX_PATH]; - CHAR8 COutputFileName[MAX_PATH]; + CHAR8 *RecordListFile; + CHAR8 *PkgOutputFileName; + CHAR8 *COutputFileName; bool CreateRecordListFile; bool CreateIfrPkgFile; CHAR8 OutputDirectory[MAX_PATH]; - CHAR8 PreprocessorOutputFileName[MAX_PATH]; + CHAR8 *PreprocessorOutputFileName; CHAR8 VfrBaseFileName[MAX_PATH]; // name of input VFR file with no path or extension CHAR8 *IncludePaths; bool SkipCPreprocessor; diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp index 1ab95be..24b0bfa 100644 --- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp +++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp @@ -1474,6 +1474,10 @@ CVfrDataStorage::GetFreeVarStoreId ( } } + if (Index == EFI_FREE_VARSTORE_ID_BITMAP_SIZE) { + return EFI_VARSTORE_ID_INVALID; + } + for (Offset = 0, Mask = 0x80000000; Mask != 0; Mask >>= 1, Offset++) { if ((mFreeVarStoreIdBitMap[Index] & Mask) == 0) { mFreeVarStoreIdBitMap[Index] |= Mask; @@ -2437,6 +2441,10 @@ CVfrQuestionDB::GetFreeQuestionId ( } } + if (Index == EFI_FREE_QUESTION_ID_BITMAP_SIZE) { + return EFI_QUESTION_ID_INVALID; + } + for (Offset = 0, Mask = 0x80000000; Mask != 0; Mask >>= 1, Offset++) { if ((mFreeQIdBitMap[Index] & Mask) == 0) { mFreeQIdBitMap[Index] |= Mask; -- 1.9.5.msysgit.0