From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 580AA21A09104 for ; Mon, 22 May 2017 18:19:32 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 May 2017 18:19:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,379,1491289200"; d="scan'208";a="971868147" Received: from shwdeopenpsi014.ccr.corp.intel.com ([10.239.9.13]) by orsmga003.jf.intel.com with ESMTP; 22 May 2017 18:19:30 -0700 From: Hao Wu To: edk2-devel@lists.01.org Cc: Hao Wu , Jiewen Yao , Liming Gao , Michael Kinney Date: Tue, 23 May 2017 09:19:25 +0800 Message-Id: <20170523011925.6264-3-hao.a.wu@intel.com> X-Mailer: git-send-email 2.12.0.windows.1 In-Reply-To: <20170523011925.6264-1-hao.a.wu@intel.com> References: <20170523011925.6264-1-hao.a.wu@intel.com> Subject: [PATCH 2/2] MdeModulePkg/PrintLib: Avoid reading content beyond the format string X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 May 2017 01:19:32 -0000 In functions DxePrintLibPrint2ProtocolVaListToBaseList() and InternalPrintLibSPrintMarker(), when processing ASCII format strings, if the format string walker pointer 'Format' is pointing at the end of the format string (i.e. '\0'), the following expression: *(Format + 1) will read an undefined value. Though this value won't affect the functionality, since it will be masked by variable 'FormatMask': (*(Format + 1) << 8)) & FormatMask (FormatMask is 0xff for ASCII format string) This commit adds additional logic to avoid reading undefined content. Cc: Jiewen Yao Cc: Liming Gao Cc: Michael Kinney Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu --- MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 66 ++++++++++++++++---- 1 file changed, 55 insertions(+), 11 deletions(-) diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c index 9f702c4fef..342eee42fc 100644 --- a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c +++ b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c @@ -130,7 +130,11 @@ DxePrintLibPrint2ProtocolVaListToBaseList ( // // Get the first character from the format string // - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } while (FormatCharacter != 0) { if (FormatCharacter == '%') { @@ -148,7 +152,11 @@ DxePrintLibPrint2ProtocolVaListToBaseList ( // // Get the next character from the format string // - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } switch (FormatCharacter) { case '.': @@ -239,7 +247,11 @@ DxePrintLibPrint2ProtocolVaListToBaseList ( // // Get the next character from the format string // - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } } return TRUE; } @@ -1596,7 +1608,11 @@ InternalPrintLibSPrintMarker ( // // Get the first character from the format string // - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } // // Loop until the end of the format string is reached or the output buffer is full @@ -1628,7 +1644,11 @@ InternalPrintLibSPrintMarker ( // for (Done = FALSE; !Done; ) { Format += BytesPerFormatCharacter; - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } switch (FormatCharacter) { case '.': Flags |= PRECISION; @@ -1681,7 +1701,11 @@ InternalPrintLibSPrintMarker ( for (Count = 0; ((FormatCharacter >= '0') && (FormatCharacter <= '9')); ){ Count = (Count * 10) + FormatCharacter - '0'; Format += BytesPerFormatCharacter; - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } } Format -= BytesPerFormatCharacter; if ((Flags & PRECISION) == 0) { @@ -1960,7 +1984,11 @@ InternalPrintLibSPrintMarker ( case '\r': Format += BytesPerFormatCharacter; - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } if (FormatCharacter == '\n') { // // Translate '\r\n' to '\r\n' @@ -1981,7 +2009,11 @@ InternalPrintLibSPrintMarker ( // ArgumentString = "\r\n"; Format += BytesPerFormatCharacter; - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } if (FormatCharacter != '\r') { Format -= BytesPerFormatCharacter; } @@ -2000,7 +2032,11 @@ InternalPrintLibSPrintMarker ( case '\r': Format += BytesPerFormatCharacter; - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } if (FormatCharacter == '\n') { // // Translate '\r\n' to '\r\n' @@ -2021,7 +2057,11 @@ InternalPrintLibSPrintMarker ( // ArgumentString = "\r\n"; Format += BytesPerFormatCharacter; - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } if (FormatCharacter != '\r') { Format -= BytesPerFormatCharacter; } @@ -2149,7 +2189,11 @@ InternalPrintLibSPrintMarker ( // // Get the next character from the format string // - FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + if (BytesPerFormatCharacter == 1) { + FormatCharacter = (*Format & 0xff) & FormatMask; + } else { + FormatCharacter = ((*Format & 0xff) | (*(Format + 1) << 8)) & FormatMask; + } } if ((Flags & COUNT_ONLY_NO_PRINT) != 0) { -- 2.12.0.windows.1