From 164a9b6752a63fca7d91ca0dcf84c0b4aa7a243d Mon Sep 17 00:00:00 2001 From: lzeng14 Date: Tue, 21 May 2013 02:22:02 +0000 Subject: [PATCH] Fix the TOCTOU issue of CommBufferSize itself for SMM communicate handler input. Signed-off-by: Star Zeng Reviewed-by: Jiewen Yao git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@14379 6f19259b-4bc3-4df7-8a09-765794883524 --- .../SmmCorePerformanceLib.c | 16 +++++++++---- .../FirmwarePerformanceSmm.c | 7 ++++-- .../FaultTolerantWriteSmm.c | 10 ++++---- .../Universal/LockBox/SmmLockBox/SmmLockBox.c | 24 +++++++++++++------ .../Variable/RuntimeDxe/VariableSmm.c | 11 +++++---- .../RuntimeDxe/VariableSmm.c | 11 +++++---- 6 files changed, 53 insertions(+), 26 deletions(-) diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c index 2bfd62a2b9..f95079bd27 100644 --- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c +++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c @@ -543,6 +543,7 @@ SmmPerformanceHandlerEx ( GAUGE_DATA_ENTRY_EX *GaugeDataEx; UINTN NumberOfEntries; UINTN LogEntryKey; + UINTN TempCommBufferSize; GaugeEntryExArray = NULL; @@ -553,11 +554,13 @@ SmmPerformanceHandlerEx ( return EFI_SUCCESS; } - if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE_EX)) { + TempCommBufferSize = *CommBufferSize; + + if(TempCommBufferSize < sizeof (SMM_PERF_COMMUNICATE_EX)) { return EFI_SUCCESS; } - if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) { + if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) { DEBUG ((EFI_D_ERROR, "SmmPerformanceHandlerEx: SMM communcation data buffer in SMRAM or overflow!\n")); return EFI_SUCCESS; } @@ -649,7 +652,8 @@ SmmPerformanceHandler ( GAUGE_DATA_ENTRY *GaugeData; UINTN NumberOfEntries; UINTN LogEntryKey; - + UINTN TempCommBufferSize; + GaugeEntryExArray = NULL; // @@ -659,11 +663,13 @@ SmmPerformanceHandler ( return EFI_SUCCESS; } - if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE)) { + TempCommBufferSize = *CommBufferSize; + + if(TempCommBufferSize < sizeof (SMM_PERF_COMMUNICATE)) { return EFI_SUCCESS; } - if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) { + if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) { DEBUG ((EFI_D_ERROR, "SmmPerformanceHandler: SMM communcation data buffer in SMRAM or overflow!\n")); return EFI_SUCCESS; } diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c index f3472e26f3..9c5fd4db85 100644 --- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c +++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c @@ -268,6 +268,7 @@ FpdtSmiHandler ( SMM_BOOT_RECORD_COMMUNICATE *SmmCommData; UINTN BootRecordSize; VOID *BootRecordData; + UINTN TempCommBufferSize; // // If input is invalid, stop processing this SMI @@ -276,11 +277,13 @@ FpdtSmiHandler ( return EFI_SUCCESS; } - if(*CommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) { + TempCommBufferSize = *CommBufferSize; + + if(TempCommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) { return EFI_SUCCESS; } - if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) { + if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) { DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM communication data buffer in SMRAM or overflow!\n")); return EFI_SUCCESS; } diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c index 2580d478a3..2b3a63081d 100644 --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c @@ -372,7 +372,7 @@ SmmFaultTolerantWriteHandler ( UINTN CommBufferPayloadSize; UINTN PrivateDataSize; UINTN Length; - + UINTN TempCommBufferSize; // // If input is invalid, stop processing this SMI @@ -381,13 +381,15 @@ SmmFaultTolerantWriteHandler ( return EFI_SUCCESS; } - if (*CommBufferSize < SMM_FTW_COMMUNICATE_HEADER_SIZE) { + TempCommBufferSize = *CommBufferSize; + + if (TempCommBufferSize < SMM_FTW_COMMUNICATE_HEADER_SIZE) { DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer size invalid!\n")); return EFI_SUCCESS; } - CommBufferPayloadSize = *CommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE; + CommBufferPayloadSize = TempCommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE; - if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) { + if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) { DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in SMRAM or overflow!\n")); return EFI_SUCCESS; } diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c index 4cb88106ee..ad4b2645cb 100644 --- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c +++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c @@ -321,17 +321,27 @@ SmmLockBoxHandler ( ) { EFI_SMM_LOCK_BOX_PARAMETER_HEADER *LockBoxParameterHeader; + UINTN TempCommBufferSize; DEBUG ((EFI_D_ERROR, "SmmLockBox SmmLockBoxHandler Enter\n")); + // + // If input is invalid, stop processing this SMI + // + if (CommBuffer == NULL || CommBufferSize == NULL) { + return EFI_SUCCESS; + } + + TempCommBufferSize = *CommBufferSize; + // // Sanity check // - if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) { + if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) { DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size invalid!\n")); return EFI_SUCCESS; } - if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) { + if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) { DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM or overflow!\n")); return EFI_SUCCESS; } @@ -346,35 +356,35 @@ SmmLockBoxHandler ( switch (LockBoxParameterHeader->Command) { case EFI_SMM_LOCK_BOX_COMMAND_SAVE: - if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) { + if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) { DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SAVE invalid!\n")); break; } SmmLockBoxSave ((EFI_SMM_LOCK_BOX_PARAMETER_SAVE *)(UINTN)LockBoxParameterHeader); break; case EFI_SMM_LOCK_BOX_COMMAND_UPDATE: - if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) { + if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) { DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for UPDATE invalid!\n")); break; } SmmLockBoxUpdate ((EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *)(UINTN)LockBoxParameterHeader); break; case EFI_SMM_LOCK_BOX_COMMAND_RESTORE: - if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) { + if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) { DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE invalid!\n")); break; } SmmLockBoxRestore ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *)(UINTN)LockBoxParameterHeader); break; case EFI_SMM_LOCK_BOX_COMMAND_SET_ATTRIBUTES: - if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) { + if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) { DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SET_ATTRIBUTES invalid!\n")); break; } SmmLockBoxSetAttributes ((EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *)(UINTN)LockBoxParameterHeader); break; case EFI_SMM_LOCK_BOX_COMMAND_RESTORE_ALL_IN_PLACE: - if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) { + if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) { DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE_ALL_IN_PLACE invalid!\n")); break; } diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c index aea9d4bcfe..1ffa74e6cc 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c @@ -501,6 +501,7 @@ SmmVariableHandler ( UINTN InfoSize; UINTN NameBufferSize; UINTN CommBufferPayloadSize; + UINTN TempCommBufferSize; // // If input is invalid, stop processing this SMI @@ -509,17 +510,19 @@ SmmVariableHandler ( return EFI_SUCCESS; } - if (*CommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) { + TempCommBufferSize = *CommBufferSize; + + if (TempCommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) { DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer size invalid!\n")); return EFI_SUCCESS; } - CommBufferPayloadSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; + CommBufferPayloadSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; if (CommBufferPayloadSize > mVariableBufferPayloadSize) { DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer payload size invalid!\n")); return EFI_SUCCESS; } - if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) { + if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) { DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer in SMRAM or overflow!\n")); return EFI_SUCCESS; } @@ -699,7 +702,7 @@ SmmVariableHandler ( case SMM_VARIABLE_FUNCTION_GET_STATISTICS: VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data; - InfoSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; + InfoSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; // // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here. diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c index cf866cecba..0be4f254d7 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c @@ -506,6 +506,7 @@ SmmVariableHandler ( UINTN InfoSize; UINTN NameBufferSize; UINTN CommBufferPayloadSize; + UINTN TempCommBufferSize; // // If input is invalid, stop processing this SMI @@ -514,17 +515,19 @@ SmmVariableHandler ( return EFI_SUCCESS; } - if (*CommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) { + TempCommBufferSize = *CommBufferSize; + + if (TempCommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) { DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer size invalid!\n")); return EFI_SUCCESS; } - CommBufferPayloadSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; + CommBufferPayloadSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; if (CommBufferPayloadSize > mVariableBufferPayloadSize) { DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer payload size invalid!\n")); return EFI_SUCCESS; } - if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) { + if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) { DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer in SMRAM or overflow!\n")); return EFI_SUCCESS; } @@ -705,7 +708,7 @@ SmmVariableHandler ( case SMM_VARIABLE_FUNCTION_GET_STATISTICS: VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data; - InfoSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; + InfoSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; // // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here. -- 2.39.2