From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web10.3872.1587138281715298728 for ; Fri, 17 Apr 2020 08:44:41 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 134.134.136.100, mailfrom: maciej.rabeda@linux.intel.com) IronPort-SDR: fD/NYVMguDBSUQYnDQts97AOdxGjM9j0w0MgcZMg+lBnXQ07QFar4biC7Lu7UInzzdQ1Indza6 Y3HHabgrKjcg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Apr 2020 08:44:40 -0700 IronPort-SDR: RVm7R+zRkk+jSzyy+OK93bIqyhfQ2sezYCfhu8xpm8FOFgzTGy8qHhrFGn4Mb6f0/+CssXV/Ox sGklBRt0oBRQ== X-IronPort-AV: E=Sophos;i="5.72,395,1580803200"; d="scan'208";a="299645874" Received: from mrabeda-mobl.ger.corp.intel.com (HELO [10.213.30.64]) ([10.213.30.64]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Apr 2020 08:44:39 -0700 Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/Ip6Dxe: Validate source data record length To: devel@edk2.groups.io, siyuan.fu@intel.com, "michael.kubacki@outlook.com" Cc: "Wu, Jiaxin" References: From: "Maciej Rabeda" Message-ID: <9d49bec8-e607-ef7c-d42a-8f04ef697953@linux.intel.com> Date: Fri, 17 Apr 2020 17:44:35 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=gbk; format=flowed Content-Transfer-Encoding: 8bit Content-Language: pl Reviewed-by: Maciej Rabeda On 09-Apr-20 10:22, Siyuan, Fu wrote: > Reviewed-by: Siyuan Fu > >> -----Original Message----- >> From: michael.kubacki@outlook.com >> Sent: 2020Äê4ÔÂ8ÈÕ 13:47 >> To: devel@edk2.groups.io >> Cc: Fu, Siyuan ; Maciej Rabeda >> ; Wu, Jiaxin >> Subject: [PATCH v1 1/1] NetworkPkg/Ip6Dxe: Validate source data record >> length >> >> From: Michael Kubacki >> >> 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 >> Cc: Maciej Rabeda >> Cc: Jiaxin Wu >> Signed-off-by: Michael Kubacki >> --- >> 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.
>> + Copyright (c) Microsoft Corporation.
>> >> 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 > > >