Re: [edk2] [Patch 3/3] BaseTools: Add more checker in Decompress algorithm to access the valid buffer

2018-10-18 Thread Zhu, Yonghong
Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Tuesday, October 16, 2018 10:06 AM
To: edk2-devel@lists.01.org
Cc: Holtsclaw, Brent 
Subject: [edk2] [Patch 3/3] BaseTools: Add more checker in Decompress algorithm 
to access the valid buffer

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

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Holtsclaw Brent 
Signed-off-by: Liming Gao 
---
 BaseTools/Source/C/Common/Decompress.c   | 23 +++--
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 26 +++-
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/Common/Decompress.c 
b/BaseTools/Source/C/Common/Decompress.c
index 9906888..71313b1 100644
--- a/BaseTools/Source/C/Common/Decompress.c
+++ b/BaseTools/Source/C/Common/Decompress.c
@@ -194,12 +194,16 @@ Returns:
   UINT16  Avail;
   UINT16  NextCode;
   UINT16  Mask;
+  UINT16  MaxTableLength;
 
   for (Index = 1; Index <= 16; Index++) {
 Count[Index] = 0;
   }
 
   for (Index = 0; Index < NumOfChar; Index++) {
+if (BitLen[Index] > 16) {
+  return (UINT16) BAD_TABLE;
+}
 Count[BitLen[Index]]++;
   }
 
@@ -237,6 +241,7 @@ Returns:
 
   Avail = NumOfChar;
   Mask  = (UINT16) (1U << (15 - TableBits));
+  MaxTableLength = (UINT16) (1U << TableBits);
 
   for (Char = 0; Char < NumOfChar; Char++) {
 
@@ -250,6 +255,9 @@ Returns:
 if (Len <= TableBits) {
 
   for (Index = Start[Len]; Index < NextCode; Index++) {
+if (Index >= MaxTableLength) {
+  return (UINT16) BAD_TABLE;
+}
 Table[Index] = Char;
   }
 
@@ -643,10 +651,14 @@ Returns: (VOID)
 
   BytesRemain--;
   while ((INT16) (BytesRemain) >= 0) {
-Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
 if (Sd->mOutBuf >= Sd->mOrigSize) {
   return ;
 }
+if (DataIdx >= Sd->mOrigSize) {
+  Sd->mBadTableFlag = (UINT16) BAD_TABLE;
+  return ;
+}
+Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
 
 BytesRemain--;
   }
@@ -684,6 +696,7 @@ Returns:
 --*/
 {
   UINT8 *Src;
+  UINT32 CompSize;
 
   *ScratchSize  = sizeof (SCRATCH_DATA);
 
@@ -692,7 +705,13 @@ Returns:
 return EFI_INVALID_PARAMETER;
   }
 
+  CompSize = Src[0] + (Src[1] << 8) + (Src[2] << 16) + (Src[3] << 24);
   *DstSize = Src[4] + (Src[5] << 8) + (Src[6] << 16) + (Src[7] << 24);
+
+  if (SrcSize < CompSize + 8 || (CompSize + 8) < 8) {
+return EFI_INVALID_PARAMETER;
+  }
+
   return EFI_SUCCESS;
 }
 
@@ -752,7 +771,7 @@ Returns:
   CompSize  = Src[0] + (Src[1] << 8) + (Src[2] << 16) + (Src[3] << 24);
   OrigSize  = Src[4] + (Src[5] << 8) + (Src[6] << 16) + (Src[7] << 24);
 
-  if (SrcSize < CompSize + 8) {
+  if (SrcSize < CompSize + 8 || (CompSize + 8) < 8) {
 return EFI_INVALID_PARAMETER;
   }
 
diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c 
b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index b88d7da..2d6fc4c 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -1757,6 +1757,7 @@ Returns:
   SCRATCH_DATA  *Scratch;
   UINT8  *Src;
   UINT32 OrigSize;
+  UINT32 CompSize;
 
   SetUtilityName(UTILITY_NAME);
 
@@ -1765,6 +1766,7 @@ Returns:
   OutBuffer = NULL;
   Scratch   = NULL;
   OrigSize = 0;
+  CompSize = 0;
   InputLength = 0;
   InputFileName = NULL;
   OutputFileName = NULL;
@@ -2006,15 +2008,24 @@ Returns:
 }
 fwrite(OutBuffer, (size_t)(DstSize), 1, OutputFile);
   } else {
+if (InputLength < 8){
+  Error (NULL, 0, 3000, "Invalid", "The input file %s is too small.", 
InputFileName);
+  goto ERROR;
+}
 //
 // Get Compressed file original size
 //
 Src = (UINT8 *)FileBuffer;
 OrigSize  = Src[4] + (Src[5] << 8) + (Src[6] << 16) + (Src[7] << 24);
+CompSize  = Src[0] + (Src[1] << 8) + (Src[2] <<16) + (Src[3] <<24);
 
 //
 // Allocate OutputBuffer
 //
+if (InputLength < CompSize + 8 || (CompSize + 8) < 8) {
+  Error (NULL, 0, 3000, "Invalid", "The input file %s data is invalid.", 
InputFileName);
+  goto ERROR;
+}
 OutBuffer = (UINT8 *)malloc(OrigSize);
 if (OutBuffer == NULL) {
   Error (NULL, 0, 4001, "Resource:", "Memory cannot be allocated!"); @@ 
-2204,12 +2215,16 @@ Returns:
   UINT16  Mask;
   UINT16  WordOfStart;
   UINT16  WordOfCount;
+  UINT16  MaxTableLength;
 
   for (Index = 0; Index <= 16; Index++) {
 Count[Index] = 0;
   }
 
   for (Index = 0; Index < NumOfChar; Index++) {
+if (BitLen[Index] > 16) {
+  return (UINT16) BAD_TABLE;
+}
 Count[BitLen[Index]]++;
   }
 
@@ -2253,6 +2268,7 @@ Returns:
 
   Avail = NumOfChar;
   Mask  = (UINT16) (1U << (15 - TableBits));
+  MaxTableLength = (UINT16) (1U << TableBits);
 
   for (Char = 0; Char 

[edk2] [Patch 3/3] BaseTools: Add more checker in Decompress algorithm to access the valid buffer

2018-10-15 Thread Liming Gao
https://bugzilla.tianocore.org/show_bug.cgi?id=686

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Holtsclaw Brent 
Signed-off-by: Liming Gao 
---
 BaseTools/Source/C/Common/Decompress.c   | 23 +++--
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 26 +++-
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/Common/Decompress.c 
b/BaseTools/Source/C/Common/Decompress.c
index 9906888..71313b1 100644
--- a/BaseTools/Source/C/Common/Decompress.c
+++ b/BaseTools/Source/C/Common/Decompress.c
@@ -194,12 +194,16 @@ Returns:
   UINT16  Avail;
   UINT16  NextCode;
   UINT16  Mask;
+  UINT16  MaxTableLength;
 
   for (Index = 1; Index <= 16; Index++) {
 Count[Index] = 0;
   }
 
   for (Index = 0; Index < NumOfChar; Index++) {
+if (BitLen[Index] > 16) {
+  return (UINT16) BAD_TABLE;
+}
 Count[BitLen[Index]]++;
   }
 
@@ -237,6 +241,7 @@ Returns:
 
   Avail = NumOfChar;
   Mask  = (UINT16) (1U << (15 - TableBits));
+  MaxTableLength = (UINT16) (1U << TableBits);
 
   for (Char = 0; Char < NumOfChar; Char++) {
 
@@ -250,6 +255,9 @@ Returns:
 if (Len <= TableBits) {
 
   for (Index = Start[Len]; Index < NextCode; Index++) {
+if (Index >= MaxTableLength) {
+  return (UINT16) BAD_TABLE;
+}
 Table[Index] = Char;
   }
 
@@ -643,10 +651,14 @@ Returns: (VOID)
 
   BytesRemain--;
   while ((INT16) (BytesRemain) >= 0) {
-Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
 if (Sd->mOutBuf >= Sd->mOrigSize) {
   return ;
 }
+if (DataIdx >= Sd->mOrigSize) {
+  Sd->mBadTableFlag = (UINT16) BAD_TABLE;
+  return ;
+}
+Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
 
 BytesRemain--;
   }
@@ -684,6 +696,7 @@ Returns:
 --*/
 {
   UINT8 *Src;
+  UINT32 CompSize;
 
   *ScratchSize  = sizeof (SCRATCH_DATA);
 
@@ -692,7 +705,13 @@ Returns:
 return EFI_INVALID_PARAMETER;
   }
 
+  CompSize = Src[0] + (Src[1] << 8) + (Src[2] << 16) + (Src[3] << 24);
   *DstSize = Src[4] + (Src[5] << 8) + (Src[6] << 16) + (Src[7] << 24);
+
+  if (SrcSize < CompSize + 8 || (CompSize + 8) < 8) {
+return EFI_INVALID_PARAMETER;
+  }
+
   return EFI_SUCCESS;
 }
 
@@ -752,7 +771,7 @@ Returns:
   CompSize  = Src[0] + (Src[1] << 8) + (Src[2] << 16) + (Src[3] << 24);
   OrigSize  = Src[4] + (Src[5] << 8) + (Src[6] << 16) + (Src[7] << 24);
 
-  if (SrcSize < CompSize + 8) {
+  if (SrcSize < CompSize + 8 || (CompSize + 8) < 8) {
 return EFI_INVALID_PARAMETER;
   }
 
diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c 
b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index b88d7da..2d6fc4c 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -1757,6 +1757,7 @@ Returns:
   SCRATCH_DATA  *Scratch;
   UINT8  *Src;
   UINT32 OrigSize;
+  UINT32 CompSize;
 
   SetUtilityName(UTILITY_NAME);
 
@@ -1765,6 +1766,7 @@ Returns:
   OutBuffer = NULL;
   Scratch   = NULL;
   OrigSize = 0;
+  CompSize = 0;
   InputLength = 0;
   InputFileName = NULL;
   OutputFileName = NULL;
@@ -2006,15 +2008,24 @@ Returns:
 }
 fwrite(OutBuffer, (size_t)(DstSize), 1, OutputFile);
   } else {
+if (InputLength < 8){
+  Error (NULL, 0, 3000, "Invalid", "The input file %s is too small.", 
InputFileName);
+  goto ERROR;
+}
 //
 // Get Compressed file original size
 //
 Src = (UINT8 *)FileBuffer;
 OrigSize  = Src[4] + (Src[5] << 8) + (Src[6] << 16) + (Src[7] << 24);
+CompSize  = Src[0] + (Src[1] << 8) + (Src[2] <<16) + (Src[3] <<24);
 
 //
 // Allocate OutputBuffer
 //
+if (InputLength < CompSize + 8 || (CompSize + 8) < 8) {
+  Error (NULL, 0, 3000, "Invalid", "The input file %s data is invalid.", 
InputFileName);
+  goto ERROR;
+}
 OutBuffer = (UINT8 *)malloc(OrigSize);
 if (OutBuffer == NULL) {
   Error (NULL, 0, 4001, "Resource:", "Memory cannot be allocated!");
@@ -2204,12 +2215,16 @@ Returns:
   UINT16  Mask;
   UINT16  WordOfStart;
   UINT16  WordOfCount;
+  UINT16  MaxTableLength;
 
   for (Index = 0; Index <= 16; Index++) {
 Count[Index] = 0;
   }
 
   for (Index = 0; Index < NumOfChar; Index++) {
+if (BitLen[Index] > 16) {
+  return (UINT16) BAD_TABLE;
+}
 Count[BitLen[Index]]++;
   }
 
@@ -2253,6 +2268,7 @@ Returns:
 
   Avail = NumOfChar;
   Mask  = (UINT16) (1U << (15 - TableBits));
+  MaxTableLength = (UINT16) (1U << TableBits);
 
   for (Char = 0; Char < NumOfChar; Char++) {
 
@@ -2266,6 +2282,9 @@ Returns:
 if (Len <= TableBits) {
 
   for (Index = Start[Len]; Index < NextCode; Index++) {
+if (Index >= MaxTableLength) {
+  return (UINT16) BAD_TABLE;
+}
 Table[Index] = Char;
   }
 
@@ -2650,11 +2669,16 @@ Returns: (VOID)
   DataIdx = Sd->mOutBuf - DecodeP (Sd) -