Reviewed-by: jiewen....@intel.com
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, February 5, 2018 1:26 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen....@intel.com>; Gao, Liming <liming....@intel.com>;
> Wang, Jian J <jian.j.w...@intel.com>
> Subject: [PATCH v2] MdePkg/SafeString: Fix potential out-of-bound memory
> access
>
> 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 <ruiyu...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Liming Gao <liming....@intel.com>
> Cc: Jian J Wang <jian.j.w...@intel.com>
> ---
> 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.<BR>
> + Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
> 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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
