diff options
3 files changed, 61 insertions, 20 deletions
diff --git a/MdeModulePkg/Include/Guid/SmmVariableCommon.h b/MdeModulePkg/Include/Guid/SmmVariableCommon.h index c7fde00ca2..478fd056b0 100644 --- a/MdeModulePkg/Include/Guid/SmmVariableCommon.h +++ b/MdeModulePkg/Include/Guid/SmmVariableCommon.h @@ -1,7 +1,7 @@ /** @file
The file defined some common structures used for communicating between SMM variable module and SMM variable wrapper module.
-Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under
the terms and conditions of the BSD License that accompanies this distribution.
The full text of the license may be found at
@@ -87,7 +87,7 @@ typedef struct { ///
typedef struct {
EFI_GUID Guid;
- UINTN NameSize;
+ UINTN NameSize; // Return name buffer size
CHAR16 Name[1];
} SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME;
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c index 2fca25981a..c85d12d951 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c @@ -280,18 +280,21 @@ RuntimeServiceGetNextVariableName ( EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;
+ UINTN SmmCommBufPayloadSize;
if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {
return EFI_INVALID_PARAMETER;
}
- if (*VariableNameSize >= mVariableBufferSize) {
- //
- // VariableNameSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to
- // overflow to a small value and pass the check in InitCommunicateBuffer().
- // To protect against this vulnerability, return EFI_INVALID_PARAMETER if VariableNameSize is >= mVariableBufferSize.
- // And there will be further check to ensure the total size is also not > mVariableBufferSize.
- //
+ //
+ // SMM Communication Buffer max payload size
+ //
+ SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+
+ //
+ // If input string exceeds SMM payload limit. Return failure
+ //
+ if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -301,16 +304,34 @@ RuntimeServiceGetNextVariableName ( // Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + *VariableNameSize;
+ if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ //
+ // If output buffer exceed SMM payload limit. Trim output buffer to SMM payload size
+ //
+ *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
+ }
+ //
+ // Payload should be Guid + NameSize + MAX of Input & Output buffer
+ //
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (*VariableNameSize, StrSize (VariableName));
+
+
Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
if (EFI_ERROR (Status)) {
goto Done;
}
ASSERT (SmmGetNextVariableName != NULL);
+ //
+ // SMM comm buffer->NameSize is buffer size for return string
+ //
SmmGetNextVariableName->NameSize = *VariableNameSize;
+
CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);
- CopyMem (SmmGetNextVariableName->Name, VariableName, *VariableNameSize);
+ //
+ // Copy whole string
+ //
+ CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));
//
// Send data to SMM
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c index 103a12914a..f1111b3b6b 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c @@ -296,18 +296,21 @@ RuntimeServiceGetNextVariableName ( EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;
+ UINTN SmmCommBufPayloadSize;
if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {
return EFI_INVALID_PARAMETER;
}
- if (*VariableNameSize >= mVariableBufferSize) {
- //
- // VariableNameSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to
- // overflow to a small value and pass the check in InitCommunicateBuffer().
- // To protect against this vulnerability, return EFI_INVALID_PARAMETER if VariableNameSize is >= mVariableBufferSize.
- // And there will be further check to ensure the total size is also not > mVariableBufferSize.
- //
+ //
+ // SMM Communication Buffer max payload size
+ //
+ SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+
+ //
+ // If input string exceeds SMM payload limit. Return failure
+ //
+ if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -317,16 +320,33 @@ RuntimeServiceGetNextVariableName ( // Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + *VariableNameSize;
+ if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ //
+ // If output buffer exceed SMM payload limit. Trim output buffer to SMM payload size
+ //
+ *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
+ }
+ //
+ // Payload should be Guid + NameSize + MAX of Input & Output buffer
+ //
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (*VariableNameSize, StrSize (VariableName));
+
Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
if (EFI_ERROR (Status)) {
goto Done;
}
ASSERT (SmmGetNextVariableName != NULL);
+ //
+ // SMM comm buffer->NameSize is buffer size for return string
+ //
SmmGetNextVariableName->NameSize = *VariableNameSize;
+
CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);
- CopyMem (SmmGetNextVariableName->Name, VariableName, *VariableNameSize);
+ //
+ // Copy whole string
+ //
+ CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));
//
// Send data to SMM
|