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

["Laszlo Ersek" <lersek@redhat.com>]

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

> 
> 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
>