[PATCH v2 0/3] Add more checker for Tianocompress and Ueficompress(CVE FIX)

[PATCH v2 0/3] Add more checker for Tianocompress and Ueficompress(CVE FIX)

[Liming Gao]

In V2, update commit message with fixed CVE number. 

Fix CVE-2017-5731,CVE-2017-5732,CVE-2017-5733,CVE-2017-5734,CVE-2017-5735
https://bugzilla.tianocore.org/show_bug.cgi?id=686

Liming Gao (3):
  MdePkg: Add more checker in UefiDecompressLib to access the valid
    buffer only(CVE FIX)
  IntelFrameworkModulePkg: Add more checker in
    UefiTianoDecompressLib(CVE FIX)
  BaseTools: Add more checker in Decompress algorithm to access the
    valid buffer(CVE FIX)

 BaseTools/Source/C/Common/Decompress.c             | 23 +++++++++++++++++--
 BaseTools/Source/C/TianoCompress/TianoCompress.c   | 26 +++++++++++++++++++++-
 .../BaseUefiTianoCustomDecompressLib.c             | 16 +++++++++++--
 .../BaseUefiDecompressLib/BaseUefiDecompressLib.c  | 17 ++++++++++++--
 4 files changed, 75 insertions(+), 7 deletions(-)

-- 
2.10.0.windows.1

[PATCH v2 1/3] MdePkg: Add more checker in UefiDecompressLib to access the valid buffer only(CVE FIX)

[Liming Gao]

Fix CVE-2017-5731,CVE-2017-5732,CVE-2017-5733,CVE-2017-5734,CVE-2017-5735
https://bugzilla.tianocore.org/show_bug.cgi?id=686

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Holtsclaw Brent <brent.holtsclaw@intel.com>
Signed-off-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
 .../BaseUefiDecompressLib/BaseUefiDecompressLib.c       | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
index dc89157..9fc637e 100644
--- a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
+++ b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
@@ -152,6 +152,7 @@ MakeTable (
   UINT16  Mask;
   UINT16  WordOfStart;
   UINT16  WordOfCount;
+  UINT16  MaxTableLength;
 
   //
   // The maximum mapping table width supported by this internal
@@ -164,6 +165,9 @@ MakeTable (
   }
 
   for (Index = 0; Index < NumOfChar; Index++) {
+    if (BitLen[Index] > 16) {
+      return (UINT16) BAD_TABLE;
+    }
     Count[BitLen[Index]]++;
   }
 
@@ -205,6 +209,7 @@ MakeTable (
 
   Avail = NumOfChar;
   Mask  = (UINT16) (1U << (15 - TableBits));
+  MaxTableLength = (UINT16) (1U << TableBits);
 
   for (Char = 0; Char < NumOfChar; Char++) {
 
@@ -218,6 +223,9 @@ MakeTable (
     if (Len <= TableBits) {
 
       for (Index = Start[Len]; Index < NextCode; Index++) {
+        if (Index >= MaxTableLength) {
+          return (UINT16) BAD_TABLE;
+        }
         Table[Index] = Char;
       }
 
@@ -620,11 +628,16 @@ Decode (
       // Write BytesRemain of bytes into mDstBase
       //
       BytesRemain--;
+
       while ((INT16) (BytesRemain) >= 0) {
-        Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
         if (Sd->mOutBuf >= Sd->mOrigSize) {
           goto Done;
         }
+        if (DataIdx >= Sd->mOrigSize) {
+          Sd->mBadTableFlag = (UINT16) BAD_TABLE;
+          goto Done;
+        }
+        Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
 
         BytesRemain--;
       }
@@ -694,7 +707,7 @@ UefiDecompressGetInfo (
   }
 
   CompressedSize   = ReadUnaligned32 ((UINT32 *)Source);
-  if (SourceSize < (CompressedSize + 8)) {
+  if (SourceSize < (CompressedSize + 8) || (CompressedSize + 8) < 8) {
     return RETURN_INVALID_PARAMETER;
   }
 
-- 
2.10.0.windows.1

[PATCH v2 2/3] IntelFrameworkModulePkg: Add more checker in UefiTianoDecompressLib(CVE FIX)

[Liming Gao]

Fix CVE-2017-5731,CVE-2017-5732,CVE-2017-5733,CVE-2017-5734,CVE-2017-5735
https://bugzilla.tianocore.org/show_bug.cgi?id=686
To make sure the valid buffer be accessed only.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Holtsclaw Brent <brent.holtsclaw@intel.com>
Signed-off-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
 .../BaseUefiTianoCustomDecompressLib.c                   | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c b/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
index ab7987a..39999a0 100644
--- a/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
+++ b/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
@@ -143,6 +143,7 @@ MakeTable (
   UINT16  Mask;
   UINT16  WordOfStart;
   UINT16  WordOfCount;
+  UINT16  MaxTableLength;
 
   //
   // The maximum mapping table width supported by this internal
@@ -155,6 +156,9 @@ MakeTable (
   }
 
   for (Index = 0; Index < NumOfChar; Index++) {
+    if (BitLen[Index] > 16) {
+      return (UINT16) BAD_TABLE;
+    }
     Count[BitLen[Index]]++;
   }
 
@@ -196,6 +200,7 @@ MakeTable (
 
   Avail = NumOfChar;
   Mask  = (UINT16) (1U << (15 - TableBits));
+  MaxTableLength = (UINT16) (1U << TableBits);
 
   for (Char = 0; Char < NumOfChar; Char++) {
 
@@ -209,6 +214,9 @@ MakeTable (
     if (Len <= TableBits) {
 
       for (Index = Start[Len]; Index < NextCode; Index++) {
+        if (Index >= MaxTableLength) {
+          return (UINT16) BAD_TABLE;
+        }
         Table[Index] = Char;
       }
 
@@ -615,10 +623,14 @@ Decode (
       //
       BytesRemain--;
       while ((INT16) (BytesRemain) >= 0) {
-        Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
         if (Sd->mOutBuf >= Sd->mOrigSize) {
           goto Done ;
         }
+        if (DataIdx >= Sd->mOrigSize) {
+          Sd->mBadTableFlag = (UINT16) BAD_TABLE;
+          goto Done ;
+        }
+        Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
 
         BytesRemain--;
       }
@@ -688,7 +700,7 @@ UefiDecompressGetInfo (
   }
 
   CompressedSize   = ReadUnaligned32 ((UINT32 *)Source);
-  if (SourceSize < (CompressedSize + 8)) {
+  if (SourceSize < (CompressedSize + 8) || (CompressedSize + 8) < 8) {
     return RETURN_INVALID_PARAMETER;
   }
 
-- 
2.10.0.windows.1

[PATCH v2 3/3] BaseTools: Add more checker in Decompress algorithm to access the valid buffer(CVE FIX)

[Liming Gao]

Fix CVE-2017-5731,CVE-2017-5732,CVE-2017-5733,CVE-2017-5734,CVE-2017-5735
https://bugzilla.tianocore.org/show_bug.cgi?id=686

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Holtsclaw Brent <brent.holtsclaw@intel.com>
Signed-off-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
 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) - 1;
 
       BytesRemain--;
+
       while ((INT16) (BytesRemain) >= 0) {
-        Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
         if (Sd->mOutBuf >= Sd->mOrigSize) {
           goto Done ;
         }
+        if (DataIdx >= Sd->mOrigSize) {
+          Sd->mBadTableFlag = (UINT16) BAD_TABLE;
+          goto Done ;
+        }
+        Sd->mDstBase[Sd->mOutBuf++] = Sd->mDstBase[DataIdx++];
 
         BytesRemain--;
       }
-- 
2.10.0.windows.1

Re: [PATCH v2 0/3] Add more checker for Tianocompress and Ueficompress(CVE FIX)

[Laszlo Ersek]

Hi Liming,

On 10/22/18 17:18, Liming Gao wrote:
> In V2, update commit message with fixed CVE number. 
> 
> Fix CVE-2017-5731,CVE-2017-5732,CVE-2017-5733,CVE-2017-5734,CVE-2017-5735
> https://bugzilla.tianocore.org/show_bug.cgi?id=686
> 
> Liming Gao (3):
>   MdePkg: Add more checker in UefiDecompressLib to access the valid
>     buffer only(CVE FIX)
>   IntelFrameworkModulePkg: Add more checker in
>     UefiTianoDecompressLib(CVE FIX)
>   BaseTools: Add more checker in Decompress algorithm to access the
>     valid buffer(CVE FIX)
> 
>  BaseTools/Source/C/Common/Decompress.c             | 23 +++++++++++++++++--
>  BaseTools/Source/C/TianoCompress/TianoCompress.c   | 26 +++++++++++++++++++++-
>  .../BaseUefiTianoCustomDecompressLib.c             | 16 +++++++++++--
>  .../BaseUefiDecompressLib/BaseUefiDecompressLib.c  | 17 ++++++++++++--
>  4 files changed, 75 insertions(+), 7 deletions(-)
> 

in the subject lines, please add a space character before the string
"(CVE FIX)". This can be done before pushing, of course.

I haven't reviewed the patches for correctness, but formally, they look
OK to me. I'm ACKing the set to confirm that. Thanks for the commit
message updates.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Laszlo

Re: [PATCH v2 0/3] Add more checker for Tianocompress and Ueficompress(CVE FIX)

[Gao, Liming]

Thank you. I will update title when push this change. 

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, October 23, 2018 6:32 PM
> To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v2 0/3] Add more checker for Tianocompress and Ueficompress(CVE FIX)
> 
> Hi Liming,
> 
> On 10/22/18 17:18, Liming Gao wrote:
> > In V2, update commit message with fixed CVE number.
> >
> > Fix CVE-2017-5731,CVE-2017-5732,CVE-2017-5733,CVE-2017-5734,CVE-2017-5735
> > https://bugzilla.tianocore.org/show_bug.cgi?id=686
> >
> > Liming Gao (3):
> >   MdePkg: Add more checker in UefiDecompressLib to access the valid
> >     buffer only(CVE FIX)
> >   IntelFrameworkModulePkg: Add more checker in
> >     UefiTianoDecompressLib(CVE FIX)
> >   BaseTools: Add more checker in Decompress algorithm to access the
> >     valid buffer(CVE FIX)
> >
> >  BaseTools/Source/C/Common/Decompress.c             | 23 +++++++++++++++++--
> >  BaseTools/Source/C/TianoCompress/TianoCompress.c   | 26 +++++++++++++++++++++-
> >  .../BaseUefiTianoCustomDecompressLib.c             | 16 +++++++++++--
> >  .../BaseUefiDecompressLib/BaseUefiDecompressLib.c  | 17 ++++++++++++--
> >  4 files changed, 75 insertions(+), 7 deletions(-)
> >
> 
> in the subject lines, please add a space character before the string
> "(CVE FIX)". This can be done before pushing, of course.
> 
> I haven't reviewed the patches for correctness, but formally, they look
> OK to me. I'm ACKing the set to confirm that. Thanks for the commit
> message updates.
> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> 
> Laszlo