Re: [edk2-devel] [PATCH v2 1/1] MdePkg : UefiFileHandleLib: fix buffer overrun in FileHandleReadLine()

["Vladimir Olovyannikov" <vladimir.olovyannikov@broadcom.com>]

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Monday, August 24, 2020 9:52 AM
> To: devel@edk2.groups.io; zhiguang.liu@intel.com;
> vladimir.olovyannikov@broadcom.com
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg : UefiFileHandleLib: fix
> buffer overrun in FileHandleReadLine()
>
> On 08/24/20 18:18, Laszlo Ersek wrote:
> > On 07/03/20 04:30, Zhiguang Liu wrote:
> >> Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
> >
> > Merged as commit 4535fc312b76, via
> > <https://github.com/tianocore/edk2/pull/896>.
>
> The commit message does not mention a TianoCore BZ. If there *is* an
> associated TianoCore BZ, please set it to RESOLVED|FIXED now, and also
> mark the above commit hash in a comment on it.
>
> Thanks
> Laszlo
Thank you Laszlo.
I modified the BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2783 as you
suggested.

Thank you,
Vladimir
>
> >
> > Thanks,
> > Laszlo
> >
> >>
> >>> -----Original Message-----
> >>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> >>> Vladimir Olovyannikov via groups.io
> >>> Sent: Thursday, July 2, 2020 10:31 AM
> >>> To: devel@edk2.groups.io
> >>> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>;
> >>> Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> >>> <liming.gao@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>
> >>> Subject: [edk2-devel] [PATCH v2 1/1] MdePkg : UefiFileHandleLib: fix
> >>> buffer overrun in FileHandleReadLine()
> >>>
> >>> If the size of the supplied buffer in FileHandleReadLine(), module
> >>> UefiFileHandleLib.c, was not 0, but was not enough to fit in the
> >>> line, the size is increased, and then the Buffer of the new size is
> >>> zeroed. This size is always larger than the supplied buffer size,
> >>> causing supplied buffer overrun. Fix the issue by using the supplied
> >>> buffer size in ZeroMem().
> >>>
> >>> Signed-off-by: Vladimir Olovyannikov
> >>> <vladimir.olovyannikov@broadcom.com>
> >>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >>> Cc: Liming Gao <liming.gao@intel.com>
> >>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> >>> ---
> >>>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 6 ++++--
> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> >>> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> >>> index 28e28e5f67d5..ab34e6ccd5f4 100644
> >>> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> >>> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> >>> @@ -969,6 +969,7 @@ FileHandleReadLine(
> >>>    UINTN       CharSize;
> >>>
> >>>    UINTN       CountSoFar;
> >>>
> >>>    UINTN       CrCount;
> >>>
> >>> +  UINTN       OldSize;
> >>>
> >>>    UINT64      OriginalFilePosition;
> >>>
> >>>
> >>>
> >>>    if (Handle == NULL
> >>>
> >>> @@ -1039,10 +1040,11 @@ FileHandleReadLine(
> >>>    // if we ran out of space tell when...
> >>>
> >>>    //
> >>>
> >>>    if ((CountSoFar+1-CrCount)*sizeof(CHAR16) > *Size){
> >>>
> >>> +    OldSize = *Size;
> >>>
> >>>      *Size = (CountSoFar+1-CrCount)*sizeof(CHAR16);
> >>>
> >>>      if (!Truncate) {
> >>>
> >>> -      if (Buffer != NULL && *Size != 0) {
> >>>
> >>> -        ZeroMem(Buffer, *Size);
> >>>
> >>> +      if (Buffer != NULL && OldSize != 0) {
> >>>
> >>> +        ZeroMem(Buffer, OldSize);
> >>>
> >>>        }
> >>>
> >>>        FileHandleSetPosition(Handle, OriginalFilePosition);
> >>>
> >>>        return (EFI_BUFFER_TOO_SMALL);
> >>>
> >>> --
> >>> 2.26.2.266.ge870325ee8
> >