Reviewed-by: Siyuan Fu <siyuan...@intel.com> > -----Original Message----- > From: michael.kuba...@outlook.com <michael.kuba...@outlook.com> > Sent: 2020年4月8日 13:47 > To: devel@edk2.groups.io > Cc: Fu, Siyuan <siyuan...@intel.com>; Maciej Rabeda > <maciej.rab...@linux.intel.com>; Wu, Jiaxin <jiaxin...@intel.com> > Subject: [PATCH v1 1/1] NetworkPkg/Ip6Dxe: Validate source data record > length > > From: Michael Kubacki <michael.kuba...@microsoft.com> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2273 > > Ip6ConfigReadConfigData() reads configuration data from a UEFI variable > and copies the data to another buffer. This change checks that the length > of the data record being copied does not exceed the size of the source > UEFI variable data buffer. > > If the size is exceeded, this change follows existing logic to treat the > variable as corrupted and deletes the variable so it will be set again. > > Cc: Siyuan Fu <siyuan...@intel.com> > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com> > Cc: Jiaxin Wu <jiaxin...@intel.com> > Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> > --- > NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c | 47 +++++++++++++------- > 1 file changed, 30 insertions(+), 17 deletions(-) > > diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c > b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c > index eb2a80b64f15..ab3801336912 100644 > --- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c > +++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c > @@ -2,6 +2,7 @@ > The implementation of EFI IPv6 Configuration Protocol. > > Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR> > + Copyright (c) Microsoft Corporation.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -390,24 +391,9 @@ Ip6ConfigReadConfigData ( > ); > if (EFI_ERROR (Status) || (UINT16) (~NetblockChecksum ((UINT8 *) > Variable, (UINT32) VarSize)) != 0) { > // > - // GetVariable still error or the variable is corrupted. > - // Fall back to the default value. > + // GetVariable error or the variable is corrupted. > // > - FreePool (Variable); > - > - // > - // Remove the problematic variable and return EFI_NOT_FOUND, a new > - // variable will be set again. > - // > - gRT->SetVariable ( > - VarName, > - &gEfiIp6ConfigProtocolGuid, > - IP6_CONFIG_VARIABLE_ATTRIBUTE, > - 0, > - NULL > - ); > - > - return EFI_NOT_FOUND; > + goto Error; > } > > // > @@ -432,7 +418,12 @@ Ip6ConfigReadConfigData ( > if (!DATA_ATTRIB_SET (DataItem->Attribute, DATA_ATTRIB_SIZE_FIXED)) { > // > // This data item has variable length data. > + // Check that the length is contained within the variable before > allocating. > // > + if (DataRecord.DataSize > VarSize - DataRecord.Offset) { > + goto Error; > + } > + > DataItem->Data.Ptr = AllocatePool (DataRecord.DataSize); > if (DataItem->Data.Ptr == NULL) { > // > @@ -454,6 +445,28 @@ Ip6ConfigReadConfigData ( > } > > return Status; > + > +Error: > + // > + // Fall back to the default value. > + // > + if (Variable != NULL) { > + FreePool (Variable); > + } > + > + // > + // Remove the problematic variable and return EFI_NOT_FOUND, a new > + // variable will be set again. > + // > + gRT->SetVariable ( > + VarName, > + &gEfiIp6ConfigProtocolGuid, > + IP6_CONFIG_VARIABLE_ATTRIBUTE, > + 0, > + NULL > + ); > + > + return EFI_NOT_FOUND; > } > > /** > -- > 2.16.3.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57104): https://edk2.groups.io/g/devel/message/57104 Mute This Topic: https://groups.io/mt/72868597/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-