From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (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 91ED52215BDB8 for ; Sun, 4 Feb 2018 21:20:32 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Feb 2018 21:26:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,463,1511856000"; d="scan'208";a="201346440" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.19]) by fmsmga006.fm.intel.com with ESMTP; 04 Feb 2018 21:26:11 -0800 From: Ruiyu Ni To: edk2-devel@lists.01.org Cc: Jiewen Yao , Liming Gao , Jian J Wang Date: Mon, 5 Feb 2018 13:26:10 +0800 Message-Id: <20180205052610.203088-1-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 Subject: [PATCH v2] MdePkg/SafeString: Fix potential out-of-bound memory access X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Feb 2018 05:20:33 -0000 Today's implementation of [Ascii]StrnCpyS/[Ascii]StrnCatS calls StrnLenS () to get the length of source string but supplies the destination buffer size as max size. It's a bug that may cause out-of-bound memory access. For example: StrnCpyS (Dest[10], 10, "hello", 6) -> StrnLenS ("hello", 10) //< cause out-of bound memory access In a pool guard enabled environment, when using shell to edit an existing file which contains empty line, the page fault is met. The patch fixes the four library functions to avoid such out-of-bound memory access. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Jiewen Yao Cc: Liming Gao Cc: Jian J Wang --- MdePkg/Library/BaseLib/SafeString.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c index 68c33e9b7b..29310889ca 100644 --- a/MdePkg/Library/BaseLib/SafeString.c +++ b/MdePkg/Library/BaseLib/SafeString.c @@ -1,7 +1,7 @@ /** @file Safe String functions. - Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -342,7 +342,7 @@ StrnCpyS ( // // 4. If Length is not less than DestMax, then DestMax shall be greater than StrnLenS(Source, DestMax). // - SourceLen = StrnLenS (Source, DestMax); + SourceLen = StrnLenS (Source, MIN (DestMax, Length)); if (Length >= DestMax) { SAFE_STRING_CONSTRAINT_CHECK ((DestMax > SourceLen), RETURN_BUFFER_TOO_SMALL); } @@ -361,7 +361,7 @@ StrnCpyS ( // pointed to by Destination. If no null character was copied from Source, then Destination[Length] is set to a null // character. // - while ((*Source != 0) && (SourceLen > 0)) { + while ((SourceLen > 0) && (*Source != 0)) { *(Destination++) = *(Source++); SourceLen--; } @@ -551,7 +551,7 @@ StrnCatS ( // // 5. If Length is not less than CopyLen, then CopyLen shall be greater than StrnLenS(Source, CopyLen). // - SourceLen = StrnLenS (Source, CopyLen); + SourceLen = StrnLenS (Source, MIN (CopyLen, Length)); if (Length >= CopyLen) { SAFE_STRING_CONSTRAINT_CHECK ((CopyLen > SourceLen), RETURN_BUFFER_TOO_SMALL); } @@ -572,7 +572,7 @@ StrnCatS ( // a null character. // Destination = Destination + DestLen; - while ((*Source != 0) && (SourceLen > 0)) { + while ((SourceLen > 0) && (*Source != 0)) { *(Destination++) = *(Source++); SourceLen--; } @@ -1916,7 +1916,7 @@ AsciiStrnCpyS ( // // 4. If Length is not less than DestMax, then DestMax shall be greater than AsciiStrnLenS(Source, DestMax). // - SourceLen = AsciiStrnLenS (Source, DestMax); + SourceLen = AsciiStrnLenS (Source, MIN (DestMax, Length)); if (Length >= DestMax) { SAFE_STRING_CONSTRAINT_CHECK ((DestMax > SourceLen), RETURN_BUFFER_TOO_SMALL); } @@ -1935,7 +1935,7 @@ AsciiStrnCpyS ( // pointed to by Destination. If no null character was copied from Source, then Destination[Length] is set to a null // character. // - while ((*Source != 0) && (SourceLen > 0)) { + while ((SourceLen > 0) && (*Source != 0)) { *(Destination++) = *(Source++); SourceLen--; } @@ -2115,7 +2115,7 @@ AsciiStrnCatS ( // // 5. If Length is not less than CopyLen, then CopyLen shall be greater than AsciiStrnLenS(Source, CopyLen). // - SourceLen = AsciiStrnLenS (Source, CopyLen); + SourceLen = AsciiStrnLenS (Source, MIN (CopyLen, Length)); if (Length >= CopyLen) { SAFE_STRING_CONSTRAINT_CHECK ((CopyLen > SourceLen), RETURN_BUFFER_TOO_SMALL); } @@ -2136,7 +2136,7 @@ AsciiStrnCatS ( // a null character. // Destination = Destination + DestLen; - while ((*Source != 0) && (SourceLen > 0)) { + while ((SourceLen > 0) && (*Source != 0)) { *(Destination++) = *(Source++); SourceLen--; } -- 2.16.1.windows.1