Besides, I recommend to separate the patch for HttpDxe and HttpUtilitiesDxe.
Thanks, Jiaxin > -----Original Message----- > From: Fu, Siyuan > Sent: Tuesday, September 25, 2018 11:43 AM > To: Li, Songpeng <[email protected]>; [email protected] > Cc: Wu, Jiaxin <[email protected]> > Subject: RE: [PATCH] NetworkPkg: fix read memory access overflow in > HTTPBoot. > > Hi, Songpeng > > The change is ok with me while I have one comment for the original > AllocateZeroPool() in these places. Since there will be always a CopyMem() > to fill up data content to the new allocated buffer, there is no need to use > AllocateZeroPool(), just AllocatePool() and adding null terminator should be > enough. This will save the unnecessary ZeroMem() time cost for better > performance. Thanks. > > > BestRegards > Fu Siyuan > > > > -----Original Message----- > > From: Li, Songpeng > > Sent: Tuesday, September 25, 2018 11:29 AM > > To: [email protected] > > Cc: Fu, Siyuan <[email protected]>; Wu, Jiaxin <[email protected]> > > Subject: [PATCH] NetworkPkg: fix read memory access overflow in > HTTPBoot. > > > > The input param String of AsciiStrStr() requires a pointer to > > Null-terminated string, however in HttpTcpReceiveHeader() and > > HttpUtilitiesParse(), the Buffersize before AllocateZeroPool() > > is equal to the size of TCP header, after the CopyMem(), it > > might not end with Null-terminator. It might cause memory > > access overflow. > > > > Cc: Fu Siyuan <[email protected]> > > Cc: Wu Jiaxin <[email protected]> > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1204 > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Songpeng Li <[email protected]> > > --- > > NetworkPkg/HttpDxe/HttpProto.c | 4 ++-- > > NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesProtocol.c | 4 +++- > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/NetworkPkg/HttpDxe/HttpProto.c > > b/NetworkPkg/HttpDxe/HttpProto.c > > index 94f89f5665..c729f76eff 100644 > > --- a/NetworkPkg/HttpDxe/HttpProto.c > > +++ b/NetworkPkg/HttpDxe/HttpProto.c > > @@ -1917,7 +1917,7 @@ HttpTcpReceiveHeader ( > > // Append the response string. > > // > > *BufferSize = *SizeofHeaders + Fragment.Len; > > - Buffer = AllocateZeroPool (*BufferSize); > > + Buffer = AllocateZeroPool (*BufferSize + 1); > > if (Buffer == NULL) { > > Status = EFI_OUT_OF_RESOURCES; > > return Status; > > @@ -2016,7 +2016,7 @@ HttpTcpReceiveHeader ( > > // Append the response string. > > // > > *BufferSize = *SizeofHeaders + Fragment.Len; > > - Buffer = AllocateZeroPool (*BufferSize); > > + Buffer = AllocateZeroPool (*BufferSize + 1); > > if (Buffer == NULL) { > > Status = EFI_OUT_OF_RESOURCES; > > return Status; > > diff --git a/NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesProtocol.c > > b/NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesProtocol.c > > index a9a1c7c586..2292b52537 100644 > > --- a/NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesProtocol.c > > +++ b/NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesProtocol.c > > @@ -298,6 +298,7 @@ HttpUtilitiesParse ( > > CHAR8 *FieldName; > > CHAR8 *FieldValue; > > UINTN Index; > > + UINTN HttpBufferSize; > > > > Status = EFI_SUCCESS; > > TempHttpMessage = NULL; > > @@ -311,7 +312,8 @@ HttpUtilitiesParse ( > > return EFI_INVALID_PARAMETER; > > } > > > > - TempHttpMessage = AllocateZeroPool (HttpMessageSize); > > + HttpBufferSize = HttpMessageSize + 1; > > + TempHttpMessage = AllocateZeroPool (HttpBufferSize); > > if (TempHttpMessage == NULL) { > > return EFI_OUT_OF_RESOURCES; > > } > > -- > > 2.18.0.windows.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
