From 732d199d8f996a483f7718ba28928dae5cc354ea Mon Sep 17 00:00:00 2001 From: czhang46 Date: Thu, 2 May 2013 01:42:39 +0000 Subject: [PATCH] Fix memory overflow & VariableSize check issue for SetVariable append write. Signed-off-by: Chao Zhang Reviewed-by : Fu Siyuan Reviewed-by : Dong Guo git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@14323 6f19259b-4bc3-4df7-8a09-765794883524 --- .../RuntimeDxe/AuthService.c | 33 ++++++++---- .../RuntimeDxe/AuthService.h | 20 +++++--- .../RuntimeDxe/Variable.c | 50 ++++++++++++++----- 3 files changed, 73 insertions(+), 30 deletions(-) diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c index 6f8808a756..440ede9144 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c @@ -192,7 +192,7 @@ AutenticatedVariableServiceInitialize ( // // Reserved runtime buffer for "Append" operation in virtual mode. // - mStorageArea = AllocateRuntimePool (PcdGet32 (PcdMaxVariableSize)); + mStorageArea = AllocateRuntimePool (MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize))); if (mStorageArea == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -1316,20 +1316,24 @@ ProcessVariable ( will be appended to the original EFI_SIGNATURE_LIST, duplicate EFI_SIGNATURE_DATA will be ignored. - @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST. - @param[in] DataSize Size of Data buffer. - @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be appended. - @param[in] NewDataSize Size of NewData buffer. + @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST. + @param[in] DataSize Size of Data buffer. + @param[in] FreeBufSize Size of free data buffer + @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be appended. + @param[in] NewDataSize Size of NewData buffer. + @param[out] MergedBufSize Size of the merged buffer - @return Size of the merged buffer. + @return EFI_BUFFER_TOO_SMALL if input Data buffer overflowed **/ -UINTN +EFI_STATUS AppendSignatureList ( IN OUT VOID *Data, IN UINTN DataSize, + IN UINTN FreeBufSize, IN VOID *NewData, - IN UINTN NewDataSize + IN UINTN NewDataSize, + OUT UINTN *MergedBufSize ) { EFI_SIGNATURE_LIST *CertList; @@ -1388,15 +1392,25 @@ AppendSignatureList ( // New EFI_SIGNATURE_DATA, append it. // if (CopiedCount == 0) { + if (FreeBufSize < sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize) { + return EFI_BUFFER_TOO_SMALL; + } + // // Copy EFI_SIGNATURE_LIST header for only once. // + CopyMem (Tail, NewCertList, sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize); Tail = Tail + sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize; + FreeBufSize -= sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize; } + if (FreeBufSize < NewCertList->SignatureSize) { + return EFI_BUFFER_TOO_SMALL; + } CopyMem (Tail, NewCert, NewCertList->SignatureSize); Tail += NewCertList->SignatureSize; + FreeBufSize -= NewCertList->SignatureSize; CopiedCount++; } @@ -1416,7 +1430,8 @@ AppendSignatureList ( NewCertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) NewCertList + NewCertList->SignatureListSize); } - return (Tail - (UINT8 *) Data); + *MergedBufSize = (Tail - (UINT8 *) Data); + return EFI_SUCCESS; } /** diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h index 510030d705..1fd687b033 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h @@ -2,7 +2,7 @@ The internal header file includes the common header files, defines internal structure and functions used by AuthService module. -Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -249,20 +249,24 @@ ProcessVarWithKek ( will be appended to the original EFI_SIGNATURE_LIST, duplicate EFI_SIGNATURE_DATA will be ignored. - @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST. - @param[in] DataSize Size of Data buffer. - @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be appended. - @param[in] NewDataSize Size of NewData buffer. + @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST. + @param[in] DataSize Size of Data buffer. + @param[in] FreeBufSize Size of free data buffer + @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be appended. + @param[in] NewDataSize Size of NewData buffer. + @param[out] MergedBufSize Size of the merged buffer - @return Size of the merged buffer. + @return EFI_BUFFER_TOO_SMALL if input Data buffer overflowed **/ -UINTN +EFI_STATUS AppendSignatureList ( IN OUT VOID *Data, IN UINTN DataSize, + IN UINTN FreeBufSize, IN VOID *NewData, - IN UINTN NewDataSize + IN UINTN NewDataSize, + OUT UINTN *MergedBufSize ); /** diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c index ebe04b50f5..f10d8f7d83 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c @@ -1649,7 +1649,7 @@ UpdateVariable ( EFI_STATUS Status; VARIABLE_HEADER *NextVariable; UINTN ScratchSize; - UINTN ScratchDataSize; + UINTN MaxDataSize; UINTN NonVolatileVarableStoreSize; UINTN VarNameOffset; UINTN VarDataOffset; @@ -1664,7 +1664,6 @@ UpdateVariable ( UINTN CacheOffset; UINTN BufSize; UINTN DataOffset; - UINTN RevBufSize; if (mVariableModuleGlobal->FvbInstance == NULL) { // @@ -1713,7 +1712,7 @@ UpdateVariable ( // NextVariable = GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.VolatileVariableBase)); ScratchSize = MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize)); - ScratchDataSize = ScratchSize - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName)); + if (Variable->CurrPtr != NULL) { // @@ -1827,14 +1826,36 @@ UpdateVariable ( DataOffset = sizeof (VARIABLE_HEADER) + Variable->CurrPtr->NameSize + GET_PAD_SIZE (Variable->CurrPtr->NameSize); CopyMem (mStorageArea, (UINT8*)((UINTN) Variable->CurrPtr + DataOffset), Variable->CurrPtr->DataSize); + // + // Set Max Common Variable Data Size as default MaxDataSize + // + MaxDataSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName)); + + if ((CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) && ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))) || (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid) && (StrCmp (VariableName, EFI_KEY_EXCHANGE_KEY_NAME) == 0))) { + // // For variables with formatted as EFI_SIGNATURE_LIST, the driver shall not perform an append of // EFI_SIGNATURE_DATA values that are already part of the existing variable value. // - BufSize = AppendSignatureList (mStorageArea, Variable->CurrPtr->DataSize, Data, DataSize); + Status = AppendSignatureList ( + mStorageArea, + Variable->CurrPtr->DataSize, + MaxDataSize - Variable->CurrPtr->DataSize, + Data, + DataSize, + &BufSize + ); + if (Status == EFI_BUFFER_TOO_SMALL) { + // + // Signture List is too long, Failed to Append + // + Status = EFI_INVALID_PARAMETER; + goto Done; + } + if (BufSize == Variable->CurrPtr->DataSize) { if ((TimeStamp == NULL) || CompareTimeStamp (TimeStamp, &Variable->CurrPtr->TimeStamp)) { // @@ -1849,20 +1870,23 @@ UpdateVariable ( } else { // // For other Variables, append the new data to the end of previous data. + // Max Harware error record variable data size is different from common variable // + if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) { + MaxDataSize = PcdGet32 (PcdMaxHardwareErrorVariableSize) - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName)); + } + + if (Variable->CurrPtr->DataSize + DataSize > MaxDataSize) { + // + // Exsiting data + Appended data exceed maximum variable size limitation + // + Status = EFI_INVALID_PARAMETER; + goto Done; + } CopyMem ((UINT8*)((UINTN) mStorageArea + Variable->CurrPtr->DataSize), Data, DataSize); BufSize = Variable->CurrPtr->DataSize + DataSize; } - RevBufSize = MIN (PcdGet32 (PcdMaxVariableSize), ScratchDataSize); - if (BufSize > RevBufSize) { - // - // If variable size (previous + current) is bigger than reserved buffer in runtime, - // return EFI_OUT_OF_RESOURCES. - // - return EFI_OUT_OF_RESOURCES; - } - // // Override Data and DataSize which are used for combined data area including previous and new data. // -- 2.39.2