[edk2] [patch] BaseTools/VfrCompile: Fix segmentation fault issues

Dandan Bi posted 1 patch 7 years, 5 months ago
Failed in applying to current master (apply log)
BaseTools/Source/C/VfrCompile/VfrCompiler.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
[edk2] [patch] BaseTools/VfrCompile: Fix segmentation fault issues
Posted by Dandan Bi 7 years, 5 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=532

(1) Add NULL check before using a pointer.
(2) Use "%s" format string in DebugError function to
    avoid crash caused by incorrect input.

Cc: Bo Chen <chenbo@pdx.edu>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index ff7057a..e65a925 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -640,11 +640,11 @@ CVfrCompiler::PreProcess (
   if (mOptions.SkipCPreprocessor == TRUE) {
     goto Out;
   }
 
   if ((pVfrFile = fopen (LongFilePath (mOptions.VfrFileName), "r")) == NULL) {
-    DebugError (NULL, 0, 0001, "Error opening the input VFR file", mOptions.VfrFileName);
+    DebugError (NULL, 0, 0001, "Error opening the input VFR file", "%s", mOptions.VfrFileName);
     goto Fail;
   }
   fclose (pVfrFile);
 
   CmdLen = strlen (mPreProcessCmd) + strlen (mPreProcessOpt) + 
@@ -709,11 +709,11 @@ CVfrCompiler::Compile (
 
   gCVfrErrorHandle.SetInputFile (InFileName);
   gCVfrErrorHandle.SetWarningAsError(mOptions.WarningAsError);
 
   if ((pInFile = fopen (LongFilePath (InFileName), "r")) == NULL) {
-    DebugError (NULL, 0, 0001, "Error opening the input file", InFileName);
+    DebugError (NULL, 0, 0001, "Error opening the input file", "%s", InFileName);
     goto Fail;
   }
 
   InputInfo.CompatibleMode = mOptions.CompatibleMode;
   if (mOptions.HasOverrideClassGuid) {
@@ -839,11 +839,11 @@ CVfrCompiler::GenBinary (
     goto Fail;
   }
 
   if (mOptions.CreateIfrPkgFile == TRUE) {
     if ((pFile = fopen (LongFilePath (mOptions.PkgOutputFileName), "wb")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening file", mOptions.PkgOutputFileName);
+      DebugError (NULL, 0, 0001, "Error opening file", "%s", mOptions.PkgOutputFileName);
       goto Fail;
     }
     if (gCFormPkg.BuildPkg (pFile, &gRBuffer) != VFR_RETURN_SUCCESS) {
       fclose (pFile);
       goto Fail;
@@ -882,11 +882,11 @@ CVfrCompiler::GenCFile (
     goto Fail;
   }
   
   if (!mOptions.CreateIfrPkgFile || mOptions.CompatibleMode) {
     if ((pFile = fopen (LongFilePath (mOptions.COutputFileName), "w")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening output C file", mOptions.COutputFileName);
+      DebugError (NULL, 0, 0001, "Error opening output C file", "%s", mOptions.COutputFileName);
       goto Fail;
     }
 
     for (Index = 0; gSourceFileHeader[Index] != NULL; Index++) {
       fprintf (pFile, "%s\n", gSourceFileHeader[Index]);
@@ -923,22 +923,22 @@ CVfrCompiler::GenRecordListFile (
   CHAR8  LineBuf[MAX_VFR_LINE_LEN];
   UINT32 LineNo;
 
   InFileName = (mOptions.SkipCPreprocessor == TRUE) ? mOptions.VfrFileName : mOptions.PreprocessorOutputFileName;
 
-  if (mOptions.CreateRecordListFile == TRUE) {
+  if (mOptions.CreateRecordListFile == TRUE && InFileName != NULL && mOptions.RecordListFile != NULL) {
     if ((InFileName[0] == '\0') || (mOptions.RecordListFile[0] == '\0')) {
       return;
     }
 
     if ((pInFile = fopen (LongFilePath (InFileName), "r")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening the input VFR preprocessor output file", InFileName);
+      DebugError (NULL, 0, 0001, "Error opening the input VFR preprocessor output file", "%s", InFileName);
       return;
     }
 
     if ((pOutFile = fopen (LongFilePath (mOptions.RecordListFile), "w")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening the record list file", mOptions.RecordListFile);
+      DebugError (NULL, 0, 0001, "Error opening the record list file", "%s", mOptions.RecordListFile);
       goto Err1;
     }
 
     fprintf (pOutFile, "//\n//  VFR compiler version " VFR_COMPILER_VERSION __BUILD_VERSION "\n//\n");
     LineNo = 0;
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch] BaseTools/VfrCompile: Fix segmentation fault issues
Posted by Dong, Eric 7 years, 4 months ago
Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dandan Bi
Sent: Thursday, July 27, 2017 10:53 AM
To: edk2-devel@lists.01.org
Cc: Bo Chen <chenbo@pdx.edu>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2] [patch] BaseTools/VfrCompile: Fix segmentation fault issues

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=532

(1) Add NULL check before using a pointer.
(2) Use "%s" format string in DebugError function to
    avoid crash caused by incorrect input.

Cc: Bo Chen <chenbo@pdx.edu>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index ff7057a..e65a925 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -640,11 +640,11 @@ CVfrCompiler::PreProcess (
   if (mOptions.SkipCPreprocessor == TRUE) {
     goto Out;
   }
 
   if ((pVfrFile = fopen (LongFilePath (mOptions.VfrFileName), "r")) == NULL) {
-    DebugError (NULL, 0, 0001, "Error opening the input VFR file", mOptions.VfrFileName);
+    DebugError (NULL, 0, 0001, "Error opening the input VFR file", 
+ "%s", mOptions.VfrFileName);
     goto Fail;
   }
   fclose (pVfrFile);
 
   CmdLen = strlen (mPreProcessCmd) + strlen (mPreProcessOpt) + @@ -709,11 +709,11 @@ CVfrCompiler::Compile (
 
   gCVfrErrorHandle.SetInputFile (InFileName);
   gCVfrErrorHandle.SetWarningAsError(mOptions.WarningAsError);
 
   if ((pInFile = fopen (LongFilePath (InFileName), "r")) == NULL) {
-    DebugError (NULL, 0, 0001, "Error opening the input file", InFileName);
+    DebugError (NULL, 0, 0001, "Error opening the input file", "%s", 
+ InFileName);
     goto Fail;
   }
 
   InputInfo.CompatibleMode = mOptions.CompatibleMode;
   if (mOptions.HasOverrideClassGuid) {
@@ -839,11 +839,11 @@ CVfrCompiler::GenBinary (
     goto Fail;
   }
 
   if (mOptions.CreateIfrPkgFile == TRUE) {
     if ((pFile = fopen (LongFilePath (mOptions.PkgOutputFileName), "wb")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening file", mOptions.PkgOutputFileName);
+      DebugError (NULL, 0, 0001, "Error opening file", "%s", 
+ mOptions.PkgOutputFileName);
       goto Fail;
     }
     if (gCFormPkg.BuildPkg (pFile, &gRBuffer) != VFR_RETURN_SUCCESS) {
       fclose (pFile);
       goto Fail;
@@ -882,11 +882,11 @@ CVfrCompiler::GenCFile (
     goto Fail;
   }
   
   if (!mOptions.CreateIfrPkgFile || mOptions.CompatibleMode) {
     if ((pFile = fopen (LongFilePath (mOptions.COutputFileName), "w")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening output C file", mOptions.COutputFileName);
+      DebugError (NULL, 0, 0001, "Error opening output C file", "%s", 
+ mOptions.COutputFileName);
       goto Fail;
     }
 
     for (Index = 0; gSourceFileHeader[Index] != NULL; Index++) {
       fprintf (pFile, "%s\n", gSourceFileHeader[Index]); @@ -923,22 +923,22 @@ CVfrCompiler::GenRecordListFile (
   CHAR8  LineBuf[MAX_VFR_LINE_LEN];
   UINT32 LineNo;
 
   InFileName = (mOptions.SkipCPreprocessor == TRUE) ? mOptions.VfrFileName : mOptions.PreprocessorOutputFileName;
 
-  if (mOptions.CreateRecordListFile == TRUE) {
+  if (mOptions.CreateRecordListFile == TRUE && InFileName != NULL && 
+ mOptions.RecordListFile != NULL) {
     if ((InFileName[0] == '\0') || (mOptions.RecordListFile[0] == '\0')) {
       return;
     }
 
     if ((pInFile = fopen (LongFilePath (InFileName), "r")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening the input VFR preprocessor output file", InFileName);
+      DebugError (NULL, 0, 0001, "Error opening the input VFR 
+ preprocessor output file", "%s", InFileName);
       return;
     }
 
     if ((pOutFile = fopen (LongFilePath (mOptions.RecordListFile), "w")) == NULL) {
-      DebugError (NULL, 0, 0001, "Error opening the record list file", mOptions.RecordListFile);
+      DebugError (NULL, 0, 0001, "Error opening the record list file", 
+ "%s", mOptions.RecordListFile);
       goto Err1;
     }
 
     fprintf (pOutFile, "//\n//  VFR compiler version " VFR_COMPILER_VERSION __BUILD_VERSION "\n//\n");
     LineNo = 0;
--
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel