From 648f98d15b5811ff9cf649bda8b762d50b735798 Mon Sep 17 00:00:00 2001 From: hhuan13 Date: Wed, 21 Sep 2011 05:17:50 +0000 Subject: 1. Enhance AuthVar driver to avoid process corrupted certificate input. Signed-off-by: hhuan13 Reviewed-by: ftian git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@12398 6f19259b-4bc3-4df7-8a09-765794883524 --- .../VariableAuthenticated/RuntimeDxe/AuthService.c | 26 ++++++++++++++--- .../VariableAuthenticated/RuntimeDxe/Variable.c | 8 +++-- .../VariableAuthenticated/RuntimeDxe/VariableSmm.c | 34 +++++++++++++--------- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c index cf94182612..fc23bb5212 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c @@ -1046,19 +1046,37 @@ VerifyTimeBasedPayload ( // return EFI_SECURITY_VIOLATION; } - + // // Find out Pkcs7 SignedData which follows the EFI_VARIABLE_AUTHENTICATION_2 descriptor. // AuthInfo.Hdr.dwLength is the length of the entire certificate, including the length of the header. // - SigData = (UINT8*) ((UINTN)Data + (UINTN)(((EFI_VARIABLE_AUTHENTICATION_2 *) 0)->AuthInfo.CertData)); - SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32)(UINTN)(((WIN_CERTIFICATE_UEFI_GUID *) 0)->CertData); + SigData = (UINT8*) ((UINTN)Data + OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData)); + + // + // Sanity check to avoid corrupted certificate input. + // + if (CertData->AuthInfo.Hdr.dwLength < (UINT32)(OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData))) { + return EFI_SECURITY_VIOLATION; + } + + + + SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32)(OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData)); // // Find out the new data payload which follows Pkcs7 SignedData directly. // PayLoadPtr = (UINT8*) ((UINTN) SigData + (UINTN) SigDataSize); - PayLoadSize = DataSize - (UINTN)(((EFI_VARIABLE_AUTHENTICATION_2 *) 0)->AuthInfo.CertData) - (UINTN) SigDataSize; + + // + // Sanity check to avoid corrupted certificate input. + // + if (DataSize < (OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData)+ (UINTN) SigDataSize)) { + return EFI_SECURITY_VIOLATION; + } + + PayLoadSize = DataSize - OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) - OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData) - (UINTN) SigDataSize; // diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c index 136bafefec..df8b30a63e 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c @@ -199,7 +199,9 @@ UpdateVariableStore ( // Check if the Data is Volatile. // if (!Volatile) { - ASSERT (Fvb != NULL); + if (Fvb == NULL) { + return EFI_INVALID_PARAMETER; + } Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr); ASSERT_EFI_ERROR (Status); @@ -1048,7 +1050,9 @@ VariableGetBestLanguage ( CONST CHAR8 *Supported; CHAR8 *Buffer; - ASSERT (SupportedLanguages != NULL); + if (SupportedLanguages == NULL) { + return NULL; + } VA_START (Args, Iso639Language); while ((Language = VA_ARG (Args, CHAR8 *)) != NULL) { diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c index 52d9aa041a..197735e399 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c @@ -241,17 +241,18 @@ GetFvbCountAndBuffer ( /** Get the variable statistics information from the information buffer pointed by gVariableInfo. - @param[in, out] InfoEntry A pointer to the buffer of variable information entry. - On input, point to the variable information returned last time. if - InfoEntry->VendorGuid is zero, return the first information. - On output, point to the next variable information. - @param[in, out] InfoSize On input, the size of the variable information buffer. - On output, the returned variable information size. - - @retval EFI_SUCCESS The variable information is found and returned successfully. - @retval EFI_UNSUPPORTED No variable inoformation exists in variable driver. The - PcdVariableCollectStatistics should be set TRUE to support it. - @retval EFI_BUFFER_TOO_SMALL The buffer is too small to hold the next variable information. + @param[in, out] InfoEntry A pointer to the buffer of variable information entry. + On input, point to the variable information returned last time. if + InfoEntry->VendorGuid is zero, return the first information. + On output, point to the next variable information. + @param[in, out] InfoSize On input, the size of the variable information buffer. + On output, the returned variable information size. + + @retval EFI_SUCCESS The variable information is found and returned successfully. + @retval EFI_UNSUPPORTED No variable inoformation exists in variable driver. The + PcdVariableCollectStatistics should be set TRUE to support it. + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to hold the next variable information. + @retval EFI_INVALID_PARAMETER Input parameter is invalid. **/ EFI_STATUS @@ -265,7 +266,10 @@ SmmVariableGetStatistics ( UINTN StatisticsInfoSize; CHAR16 *InfoName; - ASSERT (InfoEntry != NULL); + if (InfoEntry == NULL) { + return EFI_INVALID_PARAMETER; + } + VariableInfo = gVariableInfo; if (VariableInfo == NULL) { return EFI_UNSUPPORTED; @@ -348,6 +352,8 @@ SmmVariableGetStatistics ( @retval EFI_WARN_INTERRUPT_SOURCE_PENDING The interrupt is still pending and other handlers should still be called. @retval EFI_INTERRUPT_PENDING The interrupt could not be quiesced. + @retval EFI_INVALID_PARAMETER Input parameter is invalid. + **/ EFI_STATUS EFIAPI @@ -366,7 +372,9 @@ SmmVariableHandler ( VARIABLE_INFO_ENTRY *VariableInfo; UINTN InfoSize; - ASSERT (CommBuffer != NULL); + if (CommBuffer == NULL) { + return EFI_INVALID_PARAMETER; + } SmmVariableFunctionHeader = (SMM_VARIABLE_COMMUNICATE_HEADER *)CommBuffer; switch (SmmVariableFunctionHeader->Function) { -- cgit v1.2.3