[Patch 1/5 v2] MdeModulePkg/DxeHttpLib: Add boundary condition check.

* [Patch 1/5 v2] MdeModulePkg/DxeHttpLib: Add boundary condition check.
@ 2017-12-26  2:36 Jiaxin Wu
  2017-12-26  4:48 ` Gary Lin
  0 siblings, 1 reply; 2+ messages in thread
From: Jiaxin Wu @ 2017-12-26  2:36 UTC (permalink / raw)
  To: edk2-devel; +Cc: Gary Lin, Ye Ting, Fu Siyuan, Wang Fan, Wu Jiaxin

v2:
* Fix GCC the build error.

This patch is to add the boundary condition check to make sure
the accessed buffer is valid.

Cc: Gary Lin <glin@suse.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wang Fan <fan.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 38 +++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index caddbb7..915b81d 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -54,11 +54,11 @@ UriPercentDecode (
   Index = 0;
   Offset = 0;
   HexStr[2] = '\0';
   while (Index < BufferLength) {
     if (Buffer[Index] == '%') {
-      if (!NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR (Buffer[Index+2])) {
+      if (Index + 1 >= BufferLength || Index + 2 >= BufferLength || !NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR (Buffer[Index+2])) {
         return EFI_INVALID_PARAMETER;
       }
       HexStr[0] = Buffer[Index+1];
       HexStr[1] = Buffer[Index+2];
       ResultBuffer[Offset] = (CHAR8) AsciiStrHexToUintn (HexStr);
@@ -1556,20 +1556,31 @@ HttpGetFieldNameAndValue (
   )
 {
   CHAR8  *FieldNameStr;
   CHAR8  *FieldValueStr;
   CHAR8  *StrPtr;
+  CHAR8  *EndofHeader;
 
   if (String == NULL || FieldName == NULL || FieldValue == NULL) {
     return NULL;
   }
 
   *FieldName    = NULL;
   *FieldValue   = NULL;
   FieldNameStr  = NULL;
   FieldValueStr = NULL;
   StrPtr        = NULL;
+  EndofHeader   = NULL;
+
+
+  //
+  // Check whether the raw HTTP header string is valid or not.
+  //
+  EndofHeader = AsciiStrStr (String, "\r\n\r\n");
+  if (EndofHeader == NULL) {
+    return NULL;
+  }
 
   //
   // Each header field consists of a name followed by a colon (":") and the field value.
   //
   FieldNameStr = String;
@@ -1583,17 +1594,36 @@ HttpGetFieldNameAndValue (
   //
   *(FieldValueStr - 1) = 0;
 
   //
   // The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
+  // Note: LWS  = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' ' or '\t'.
+  //       CRLF = '\r\n'.
+  //       SP   = ' '.
+  //       HT   = '\t' (Tab).
   //
   while (TRUE) {
     if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
+      //
+      // Boundary condition check. 
+      //
+      if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 1) {
+        return NULL;  
+      }
+      
       FieldValueStr ++;
-    } else if (*FieldValueStr == '\r' && *(FieldValueStr + 1) == '\n' &&
-               (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
-      FieldValueStr = FieldValueStr + 3;
+    } else if (*FieldValueStr == '\r') {
+      //
+      // Boundary condition check. 
+      //
+      if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 3) {
+        return NULL;  
+      }
+
+      if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
+        FieldValueStr = FieldValueStr + 3;
+      }
     } else {
       break;
     }
   }
 
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread