From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (NAM12-BN8-obe.outbound.protection.outlook.com [40.92.21.62]) by mx.groups.io with SMTP id smtpd.web10.10712.1596165355818453750 for ; Thu, 30 Jul 2020 20:15:56 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@outlook.com header.s=selector1 header.b=JaU10KmZ; spf=pass (domain: outlook.com, ip: 40.92.21.62, mailfrom: michael.kubacki@outlook.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cp9IGLcF3SMT3xO4yn+W6KdfWyU854MR379DQUinXxeBt+D28qPFfIiWVM0/QpHe2hvg1RBToRMmzwxB4NGl5udET3FDkDS8KDMTCCFKuI47FB8Q/BYHaoMqJQ2WwhWHF3stpRLPqui4xNBTkjROTFB1Lecm0aHAu015sM7LpIPAsnv3d57+mxLOq4IZG+sAQCAyTXdY5ZJNG9pyz+GN0Zud2UNt9vhh4RGBzA4IW/z60Aq++Hotd7UU4TiZJEKgXA514mLEBJMSW0Vo+l/tBztNQjMfTL+MXKSqua1K0WGQp2DrbL+bDr0zhZ817NIaS+Ru2RrdlylXhgYfzVoJBA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=qa12h+rUcfeQwB19k6ZbJZb8m/XdSpq+eGaYg6Ug1/g=; b=aK4Klf3pxEiss2RBlLf723jNGspq58oYnzdSAIguQLOyy5ZAkMn2W2eow8gRJ13Wivx1CgedNOSWfd07RT4WVFN3gruk2u9or7e6ejJn4/IgJuyrccIyPC9vIO1xJKFg+/D9yIDg86ZdTyEXUHY8USFY7E125dTICHJJwfRKw94IfhdiqyPcWTxsnwq+61UzoksTWd0tAsYpbThHvPqimEKnaWP2wfDUHWajAqr93KMXc4PhA6/Xqsg9t+Ht/hKvksuv7LwsalDFV5W8dnlA+8Z/Wo60C1urpDX+np8PgDAoywY9lgfeMynHjSJkN+hwB8aM3MrcHtJEjtIY38kbXw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=qa12h+rUcfeQwB19k6ZbJZb8m/XdSpq+eGaYg6Ug1/g=; b=JaU10KmZbH58D66C37H1eLGURK/REdOgTr4AEGGUrR7Cv7wDpQ/rVo3G7iLoE3plakcfyLXKh9+gtmG2/G2Sh3vsRr7wDo2djk+nDThJWnE8f8+a2U347Fwsa3M3N4F4OILOO92g0eAVhHX4U4IoNm62wUE5PfLTfwGN54EggfJQIPFEi987fiXGPJmt+zQnKwyUiAj/viUA3KvZNFt/pMzOvLDByE0oED0tWNSA8e1eHvPrzJC4EoBjycqEJGfLBilE2rxDMqnmLd/sEHG28R8r7czOS/5jXytb5xUy3PFzAWYnMUoidW/Is8eXpFHVBjZ85h+B3BXvGh3FDW5m2A== Received: from MW2NAM12FT019.eop-nam12.prod.protection.outlook.com (2a01:111:e400:fc65::47) by MW2NAM12HT079.eop-nam12.prod.protection.outlook.com (2a01:111:e400:fc65::314) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.14; Fri, 31 Jul 2020 03:15:54 +0000 Received: from MWHPR07MB3440.namprd07.prod.outlook.com (2a01:111:e400:fc65::4c) by MW2NAM12FT019.mail.protection.outlook.com (2a01:111:e400:fc65::86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.14 via Frontend Transport; Fri, 31 Jul 2020 03:15:54 +0000 X-IncomingTopHeaderMarker: OriginalChecksum:24247874D696429CDFCAE568C88EA42C9B1610D01D97A91531EC082941DFC9FA;UpperCasedChecksum:60C171DF68D12FA959D4BEB03AB2D7127400EB286B1269B76D2766B13CE06CF3;SizeAsReceived:7725;Count:49 Received: from MWHPR07MB3440.namprd07.prod.outlook.com ([fe80::9856:570e:1735:974e]) by MWHPR07MB3440.namprd07.prod.outlook.com ([fe80::9856:570e:1735:974e%7]) with mapi id 15.20.3216.033; Fri, 31 Jul 2020 03:15:54 +0000 From: "Michael Kubacki" To: devel@edk2.groups.io CC: Liming Gao , Michael D Kinney Subject: [PATCH v1 7/7] FmpDevicePkg/FmpDxe: Improve function parameter validation Date: Thu, 30 Jul 2020 20:14:48 -0700 Message-ID: X-Mailer: git-send-email 2.27.0.windows.1 In-Reply-To: <20200731031448.1103-1-michael.kubacki@outlook.com> References: <20200731031448.1103-1-michael.kubacki@outlook.com> X-ClientProxiedBy: CO2PR04CA0115.namprd04.prod.outlook.com (2603:10b6:104:7::17) To MWHPR07MB3440.namprd07.prod.outlook.com (2603:10b6:301:69::28) Return-Path: michael.kubacki@outlook.com X-Microsoft-Original-Message-ID: <20200731031448.1103-8-michael.kubacki@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from localhost.localdomain (2001:4898:80e8:0:8072:23b8:48ea:d2c1) by CO2PR04CA0115.namprd04.prod.outlook.com (2603:10b6:104:7::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.18 via Frontend Transport; Fri, 31 Jul 2020 03:15:54 +0000 X-Mailer: git-send-email 2.27.0.windows.1 X-Microsoft-Original-Message-ID: <20200731031448.1103-8-michael.kubacki@outlook.com> X-TMN: [wqa0rQi7jRU2yXj8xBw6VjHHCDdf4LbzS2q8RrpFNpdFXBN2HdU8X8ZZyaP1k2au] X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 49 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: a9e9ddf2-df10-4f90-6d3d-08d835000926 X-MS-TrafficTypeDiagnostic: MW2NAM12HT079: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: FSzD9Z+u6KImpw0lqZJ9AabNJp1H+pAV/g2g1yLX99UrmE7l2bv5QdBR4JRRxXCPS1i2xr03fpcRrqtEga3F53Lc4gBV4U6iyoLSFobsQW6dqn5ZMrAxUH7hFwuIZE6VXreyqK0Loh8FbeU6EDb8fIAUoEePBoYqqKRVosf+Hyb6Ag+jSxZGVjxjTR4Bkpgrg88rlHzgMSWS7bzSXfD4FCjZqP9UHixOtYlJXzl8h5m2ALJLVhrZ70RZM6xqijf1 X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:0;SRV:;IPV:NLI;SFV:NSPM;H:MWHPR07MB3440.namprd07.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:;DIR:OUT;SFP:1901; X-MS-Exchange-AntiSpam-MessageData: cLgavsJeIbkr0mBekdpbuqdF/X0ElQAEEqAmfPKyiODq7yY+LngWBb3KSeeG5OweCERSKA55nAG4s6L0rpITpr4yzwO2PQnQjcXXDKwWwDp/ZeNWt8uHEySTKxq/zmcPAWs4N7vLf09VgiCRL6Zzlp6hXu+F/qgS6d0GinoL+EKAJNwjjUOdwMyJbm5ZfNMxgoxYe6Ml8BtLlCu+LvgL5Q== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: a9e9ddf2-df10-4f90-6d3d-08d835000926 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jul 2020 03:15:54.4639 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-AuthSource: MW2NAM12FT019.eop-nam12.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW2NAM12HT079 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain From: Michael Kubacki REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2869 Makes some minor improvements to function parameter validation in FmpDxe, in particular to externally exposed functions such as those that back EFI_FIRMWARE_MANAGEMENT_PROTOCOL. Cc: Liming Gao Cc: Michael D Kinney Signed-off-by: Michael Kubacki --- FmpDevicePkg/FmpDxe/FmpDxe.c | 56 +++++++++++++++++--- FmpDevicePkg/FmpDxe/FmpDxe.h | 10 ++-- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index a3e342591936..958d9b394b71 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.c +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c @@ -278,6 +278,11 @@ PopulateDescriptor ( EFI_STATUS Status; UINT32 DependenciesSize; =20 + if (Private =3D=3D NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): PopulateDescriptor() - Private is NU= LL.\n", mImageIdName)); + return; + } + if (Private->DescriptorPopulated) { return; } @@ -429,7 +434,7 @@ PopulateDescriptor ( @retval EFI_SUCCESS The device was successfully updated w= ith the new image. @retval EFI_BUFFER_TOO_SMALL The ImageInfo buffer was too small. T= he current buffer size needed to hold the image(s) informati= on is returned in ImageInfoSize. - @retval EFI_INVALID_PARAMETER ImageInfoSize is NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_DEVICE_ERROR Valid information could not be return= ed. Possible corrupted image. =20 **/ @@ -451,6 +456,12 @@ GetTheImageInfo ( =20 Status =3D EFI_SUCCESS; =20 + if (This =3D=3D NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImageInfo() - This is NULL.\n", m= ImageIdName)); + Status =3D EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -536,7 +547,7 @@ GetTheImageInfo ( @retval EFI_BUFFER_TOO_SMALL The buffer specified by ImageSize is too = small to hold the image. The current buffer size needed to = hold the image is returned in ImageSize. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL or ImageIndex = is invalid. @retval EFI_NOT_FOUND The current image is not copied to the bu= ffer. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due = to an authentication failure. @@ -561,6 +572,12 @@ GetTheImage ( =20 Status =3D EFI_SUCCESS; =20 + if (This =3D=3D NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImage() - This is NULL.\n", mImag= eIdName)); + Status =3D EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -615,7 +632,8 @@ GetTheImage ( @param[in] Image Pointer to the image. @param[in] ImageSize Size of the image. @param[in] AdditionalHeaderSize Size of any headers that cannot be ca= lculated by this function. - @param[out] PayloadSize + @param[out] PayloadSize An optional pointer to a UINTN that h= olds the size of the payload + (image size minus headers) =20 @retval !NULL Valid pointer to the header. @retval NULL Structure is bad and pointer cannot be found. @@ -626,7 +644,7 @@ GetFmpHeader ( IN CONST EFI_FIRMWARE_IMAGE_AUTHENTICATION *Image, IN CONST UINTN ImageSize, IN CONST UINTN AdditionalHeaderSize, - OUT UINTN *PayloadSize + OUT UINTN *PayloadSize OPTIONAL ) { // @@ -640,7 +658,10 @@ GetFmpHeader ( return NULL; } =20 - *PayloadSize =3D ImageSize - (sizeof (Image->MonotonicCount) + Image->Au= thInfo.Hdr.dwLength + AdditionalHeaderSize); + if (PayloadSize !=3D NULL) { + *PayloadSize =3D ImageSize - (sizeof (Image->MonotonicCount) + Image->= AuthInfo.Hdr.dwLength + AdditionalHeaderSize); + } + return (VOID *)((UINT8 *)Image + sizeof (Image->MonotonicCount) + Image-= >AuthInfo.Hdr.dwLength + AdditionalHeaderSize); } =20 @@ -663,6 +684,10 @@ GetAllHeaderSize ( { UINT32 CalculatedSize; =20 + if (Image =3D=3D NULL) { + return 0; + } + CalculatedSize =3D sizeof (Image->MonotonicCount) + AdditionalHeaderSize + Image->AuthInfo.Hdr.dwLength; @@ -698,7 +723,7 @@ GetAllHeaderSize ( =20 @retval EFI_SUCCESS The image was successfully checked. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due = to an authentication failure. =20 @@ -743,6 +768,12 @@ CheckTheImage ( return EFI_UNSUPPORTED; } =20 + if (This =3D=3D NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - This is NULL.\n", mIm= ageIdName)); + Status =3D EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -978,7 +1009,7 @@ CheckTheImage ( =20 @retval EFI_SUCCESS The device was successfully updated with = the new image. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due = to an authentication failure. =20 @@ -1026,6 +1057,12 @@ SetTheImage ( return EFI_UNSUPPORTED; } =20 + if (This =3D=3D NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - This is NULL.\n", mI= mageIdName)); + Status =3D EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -1382,6 +1419,11 @@ FmpDxeLockEventNotify ( EFI_STATUS Status; FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private; =20 + if (Context =3D=3D NULL) { + ASSERT (Context !=3D NULL); + return; + } + Private =3D (FIRMWARE_MANAGEMENT_PRIVATE_DATA *)Context; =20 if (!Private->FmpDeviceLocked) { diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.h b/FmpDevicePkg/FmpDxe/FmpDxe.h index 30754dea495e..4dfec316a558 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.h +++ b/FmpDevicePkg/FmpDxe/FmpDxe.h @@ -3,7 +3,7 @@ image stored in a firmware device with platform and firmware device spec= ific information provided through PCDs and libraries. =20 - Copyright (c) 2016, Microsoft Corporation. All rights reserved.
+ Copyright (c) Microsoft Corporation.
Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
=20 SPDX-License-Identifier: BSD-2-Clause-Patent @@ -132,7 +132,7 @@ DetectTestKey ( @retval EFI_SUCCESS The device was successfully updated w= ith the new image. @retval EFI_BUFFER_TOO_SMALL The ImageInfo buffer was too small. T= he current buffer size needed to hold the image(s) informati= on is returned in ImageInfoSize. - @retval EFI_INVALID_PARAMETER ImageInfoSize is NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_DEVICE_ERROR Valid information could not be return= ed. Possible corrupted image. =20 **/ @@ -166,7 +166,7 @@ GetTheImageInfo ( @retval EFI_BUFFER_TOO_SMALL The buffer specified by ImageSize is too = small to hold the image. The current buffer size needed to = hold the image is returned in ImageSize. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL or ImageIndex = is invalid. @retval EFI_NOT_FOUND The current image is not copied to the bu= ffer. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due = to an authentication failure. @@ -198,7 +198,7 @@ GetTheImage ( =20 @retval EFI_SUCCESS The image was successfully checked. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due = to an authentication failure. =20 @@ -254,7 +254,7 @@ CheckTheImage ( =20 @retval EFI_SUCCESS The device was successfully updated with = the new image. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due = to an authentication failure. =20 --=20 2.27.0.windows.1