From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (NAM12-DM6-obe.outbound.protection.outlook.com [40.92.22.10]) by mx.groups.io with SMTP id smtpd.web12.387.1596663641364559646 for ; Wed, 05 Aug 2020 14:40:41 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@outlook.com header.s=selector1 header.b=lEL32rfw; spf=pass (domain: outlook.com, ip: 40.92.22.10, mailfrom: michael.kubacki@outlook.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AeC498tmPQanUdGGINf6zKM8+CmS1QfBVJwvCDAkDeGQZZeCL3vPq7ffbWkPSdsza8DlBLgQYwyXJoSY5QSX3obOFv0PB+zplIQlPsey3Z0Xy8U9dCnmFPT8Sabu8AJgdkSCerBpGaBnaYHvn3XsG27cBWIji6Cb8yKK8JxvwY0Dot4W/pC6Oj63ayZlzqtlEQ9AB3fYDyXKqdP3NnQEGiVLxRxRhtm93bkcwYzOE3IXzuJEyaw6br6rUpddzNCIdm45gDzKNt46y63/guNFlXri+ZtDGdGww+zrLfkBJpDPH1Xxw0juFxbx+AL38k0QGin2kXv5DUeLoiZm+ii55g== 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=6E094JjQGt0SLa9KudZ+8Xmmtl/RC7Dk757S7DElYE8=; b=lp3YVQtHvGshJT1HLxT1sX0JLxwKKNrGfJQ8kiqDSaVBgKExz1IhCceSJv7n5mK2zGRXH18KMuKjMTmRzl2TyU+N9uf6AwiWL6iw4km1sEROkIIGgoaldwNiJhyPQotrdma0yrQdDOIB//V4Gd3jATfudZBT0gzsWhiIW2+Eh4ejlBB3BfLgINjDhnKkUTs6gKJWOuxhNN8UcN+bstf8q8x5ximorzL12X/9UiOZvNkxYjnYmSbD+uduw4I09H+M8x22VLHOREXYzF3Id3KfRAZT+Y1XfWTj1hs8TPQHAvgCrypeccUXP/cI1Ok1Dh4+maFALGCxDl6IJpspdi/qNw== 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=6E094JjQGt0SLa9KudZ+8Xmmtl/RC7Dk757S7DElYE8=; b=lEL32rfw+dmhP5D3DHAN+aG2gmvgoQcj0Jt5UTLEX32/fEg444JRDM0lfr1kXJ9ewag2Mn5lGHNij85vyWWuHI8iQqdZP/BOKes8LqV7YSE7JYJ//R5QFCKeZ4pOUlY0vRvy5o1GgYEvNj5NZvVxo1+ePonnFCd5KCIrU1lZGbld1AweSktkkeJl0ATbXs3TXnFeuVvK/HcgcXBXSGMVuBSwQI1sOhDCeiwKAa9aGo3TKN120qHNul7K2iYyD0QJJKDtsLKN9xolph4eV+NUlwCCUeQuLzpcEwU3KGoiJbaC0V0wDQzcSKFVgO+Vyv9O9P6nZlHYcWFc84DOHAb9mw== Received: from MW2NAM12FT045.eop-nam12.prod.protection.outlook.com (2a01:111:e400:fc65::41) by MW2NAM12HT215.eop-nam12.prod.protection.outlook.com (2a01:111:e400:fc65::478) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3261.10; Wed, 5 Aug 2020 21:40:39 +0000 Received: from MWHPR07MB3440.namprd07.prod.outlook.com (2a01:111:e400:fc65::42) by MW2NAM12FT045.mail.protection.outlook.com (2a01:111:e400:fc65::268) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3261.10 via Frontend Transport; Wed, 5 Aug 2020 21:40:39 +0000 X-IncomingTopHeaderMarker: OriginalChecksum:FE6EB380B28196980F3A8108F0B93EEA39B4DD24537300AE5AA711D59C61AD84;UpperCasedChecksum:163A381FABAFE2FE0A535AC6389552738B4DDC6134804F87A369896233FBFA15;SizeAsReceived:7801;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.3239.022; Wed, 5 Aug 2020 21:40:39 +0000 From: "Michael Kubacki" To: devel@edk2.groups.io CC: Liming Gao , Michael D Kinney , Guomin Jiang , Wei6 Xu Subject: [PATCH v2 7/7] FmpDevicePkg/FmpDxe: Improve function parameter validation Date: Wed, 5 Aug 2020 14:39:44 -0700 Message-ID: X-Mailer: git-send-email 2.27.0.windows.1 In-Reply-To: <20200805213944.1811-1-michael.kubacki@outlook.com> References: <20200805213944.1811-1-michael.kubacki@outlook.com> X-ClientProxiedBy: MWHPR2001CA0014.namprd20.prod.outlook.com (2603:10b6:301:15::24) To MWHPR07MB3440.namprd07.prod.outlook.com (2603:10b6:301:69::28) Return-Path: michael.kubacki@outlook.com X-Microsoft-Original-Message-ID: <20200805213944.1811-8-michael.kubacki@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from localhost.localdomain (2001:4898:80e8:a:f5bb:f844:3092:4e93) by MWHPR2001CA0014.namprd20.prod.outlook.com (2603:10b6:301:15::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.17 via Frontend Transport; Wed, 5 Aug 2020 21:40:39 +0000 X-Mailer: git-send-email 2.27.0.windows.1 X-Microsoft-Original-Message-ID: <20200805213944.1811-8-michael.kubacki@outlook.com> X-TMN: [W2Zghu9Y+SG7UYoPI/XWfdfI6FmjiF2av6pbIZVGvl4PQlpsDi10NghKZMVFUh2H] X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 49 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: 04eb9860-180a-46f9-2ffe-08d83988320c X-MS-TrafficTypeDiagnostic: MW2NAM12HT215: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: +7Yd/i0BUlsZYlVIcnPrL9dJcouZC8ayFjU8CnjsBO7k2WjYwDNyrTxripmKEOnRTXMpfZ703WXUVbNlYcBOwzDoKZ2N65jrRAxx7ewQQCUlinrAR37LO+4NVSvhFqGeY8RhTiodwqGuCE/RRz+ylvxgKL8XOMPi2rcbF/+sbukEb2F+OzM/VCHEf7K8jjGMjUC+81+pSpqgPDAUBuZVwT8HUmdzHh/3uTMS3mCBsT8twES16/pOBRrt613YTCDd 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: XnHBjidAvZwllAR1KC6ksj89G7gOqNrP6OxadlTQ9yKvsXssAzhdvV+MKquHz2YQ0KZQzF4LbUojDikfvLicFLpxluTzD9j+xUQvnx1JPy6H2wdrG73LEC4rwNzT8vEnk1gMkh56wUEcwBVTVqDRuRNQYkm/T8oU9YqHr++3r+6/boFq5C5n99hjw1u5X794GSm/X9Vi0yqiIJLz1o0nvw== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 04eb9860-180a-46f9-2ffe-08d83988320c X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Aug 2020 21:40:39.7166 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-AuthSource: MW2NAM12FT045.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: MW2NAM12HT215 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 Cc: Guomin Jiang Cc: Wei6 Xu Signed-off-by: Michael Kubacki Reviewed-by: Michael D Kinney Reviewed-by: Guomin Jiang Reviewed-by: Wei6 Xu --- FmpDevicePkg/FmpDxe/FmpDxe.c | 59 +++++++++++++++++--- FmpDevicePkg/FmpDxe/FmpDxe.h | 10 ++-- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index a3e342591936..9fa63cff4aa2 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,11 @@ GetAllHeaderSize ( { UINT32 CalculatedSize; =20 + if (Image =3D=3D NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetAllHeaderSize() - Image is NULL.\= n", mImageIdName)); + return 0; + } + CalculatedSize =3D sizeof (Image->MonotonicCount) + AdditionalHeaderSize + Image->AuthInfo.Hdr.dwLength; @@ -698,7 +724,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 or ImageIndex = is invalid. @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 +769,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 // @@ -851,7 +883,7 @@ CheckTheImage ( if (ImageIndex !=3D 1) { DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - Image Index Invalid.\= n", mImageIdName)); *ImageUpdatable =3D IMAGE_UPDATABLE_INVALID_TYPE; - Status =3D EFI_SUCCESS; + Status =3D EFI_INVALID_PARAMETER; goto cleanup; } =20 @@ -978,7 +1010,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 or ImageIndex = is invalid. @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 +1058,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 +1420,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..7cb273a41b78 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 or ImageIndex = is invalid. @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 or ImageIndex = is invalid. @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