summaryrefslogtreecommitdiff
path: root/SecurityPkg/VariableAuthenticated/RuntimeDxe
diff options
context:
space:
mode:
Diffstat (limited to 'SecurityPkg/VariableAuthenticated/RuntimeDxe')
-rw-r--r--SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c81
-rw-r--r--SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c47
2 files changed, 110 insertions, 18 deletions
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
index 678cff3c7d..47bc390f6f 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
@@ -93,6 +93,32 @@ InternalIsAddressInSmram (
return FALSE;
}
+/**
+ This function check if the address refered by Buffer and Length is valid.
+
+ @param Buffer the buffer address to be checked.
+ @param Length the buffer length to be checked.
+
+ @retval TRUE this address is valid.
+ @retval FALSE this address is NOT valid.
+**/
+BOOLEAN
+InternalIsAddressValid (
+ IN UINTN Buffer,
+ IN UINTN Length
+ )
+{
+ if (Buffer > (MAX_ADDRESS - Length)) {
+ //
+ // Overflow happen
+ //
+ return FALSE;
+ }
+ if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {
+ return FALSE;
+ }
+ return TRUE;
+}
/**
Initializes a basic mutual exclusion lock.
@@ -423,6 +449,7 @@ SmmVariableHandler (
SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO *QueryVariableInfo;
VARIABLE_INFO_ENTRY *VariableInfo;
UINTN InfoSize;
+ UINTN NameBufferSize;
//
// If input is invalid, stop processing this SMI
@@ -435,8 +462,8 @@ SmmVariableHandler (
return EFI_SUCCESS;
}
- if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {
- DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));
+ if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM or overflow!\n"));
return EFI_SUCCESS;
}
@@ -445,6 +472,14 @@ SmmVariableHandler (
switch (SmmVariableFunctionHeader->Function) {
case SMM_VARIABLE_FUNCTION_GET_VARIABLE:
SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;
+ if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||
+ ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {
+ //
+ // Prevent InfoSize overflow happen
+ //
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)
+ SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;
@@ -457,6 +492,14 @@ SmmVariableHandler (
goto EXIT;
}
+ if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {
+ //
+ // Make sure VariableName is A Null-terminated string.
+ //
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
+
Status = VariableServiceGetVariable (
SmmVariableHeader->Name,
&SmmVariableHeader->Guid,
@@ -468,6 +511,13 @@ SmmVariableHandler (
case SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME:
GetNextVariableName = (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) SmmVariableFunctionHeader->Data;
+ if ((UINTN)(~0) - GetNextVariableName->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ //
+ // Prevent InfoSize overflow happen
+ //
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + GetNextVariableName->NameSize;
//
@@ -479,6 +529,15 @@ SmmVariableHandler (
goto EXIT;
}
+ NameBufferSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE - OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
+ if (NameBufferSize < sizeof (CHAR16) || GetNextVariableName->Name[NameBufferSize/sizeof (CHAR16) - 1] != L'\0') {
+ //
+ // Make sure input VariableName is A Null-terminated string.
+ //
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
+
Status = VariableServiceGetNextVariableName (
&GetNextVariableName->NameSize,
GetNextVariableName->Name,
@@ -488,6 +547,14 @@ SmmVariableHandler (
case SMM_VARIABLE_FUNCTION_SET_VARIABLE:
SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;
+ if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||
+ ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {
+ //
+ // Prevent InfoSize overflow happen
+ //
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)
+ SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;
@@ -501,6 +568,14 @@ SmmVariableHandler (
goto EXIT;
}
+ if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {
+ //
+ // Make sure VariableName is A Null-terminated string.
+ //
+ Status = EFI_ACCESS_DENIED;
+ goto EXIT;
+ }
+
Status = VariableServiceSetVariable (
SmmVariableHeader->Name,
&SmmVariableHeader->Guid,
@@ -555,7 +630,7 @@ SmmVariableHandler (
//
if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) {
- DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM!\n"));
Status = EFI_ACCESS_DENIED;
goto EXIT;
}
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
index 5a02b77d53..cdd407d66b 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -207,6 +207,7 @@ RuntimeServiceGetVariable (
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
UINTN SmmCommBufPayloadSize;
UINTN TempDataSize;
+ UINTN VariableNameSize;
if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {
return EFI_INVALID_PARAMETER;
@@ -221,11 +222,12 @@ RuntimeServiceGetVariable (
//
SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
TempDataSize = *DataSize;
+ VariableNameSize = StrSize (VariableName);
//
// If VariableName exceeds SMM payload limit. Return failure
//
- if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
+ if (VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -235,13 +237,13 @@ RuntimeServiceGetVariable (
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
//
- if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName)) {
+ if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize) {
//
// If output data buffer exceed SMM payload limit. Trim output buffer to SMM payload size
//
- TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName);
+ TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize;
}
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + TempDataSize;
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + VariableNameSize + TempDataSize;
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);
if (EFI_ERROR (Status)) {
@@ -251,7 +253,7 @@ RuntimeServiceGetVariable (
CopyGuid (&SmmVariableHeader->Guid, VendorGuid);
SmmVariableHeader->DataSize = TempDataSize;
- SmmVariableHeader->NameSize = StrSize (VariableName);
+ SmmVariableHeader->NameSize = VariableNameSize;
if (Attributes == NULL) {
SmmVariableHeader->Attributes = 0;
} else {
@@ -315,6 +317,8 @@ RuntimeServiceGetNextVariableName (
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;
UINTN SmmCommBufPayloadSize;
+ UINTN OutVariableNameSize;
+ UINTN InVariableNameSize;
if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {
return EFI_INVALID_PARAMETER;
@@ -324,11 +328,13 @@ RuntimeServiceGetNextVariableName (
// SMM Communication Buffer max payload size
//
SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+ OutVariableNameSize = *VariableNameSize;
+ InVariableNameSize = StrSize (VariableName);
//
// If input string exceeds SMM payload limit. Return failure
//
- if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (InVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -338,16 +344,16 @@ RuntimeServiceGetNextVariableName (
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
//
- if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (OutVariableNameSize > 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);
+ OutVariableNameSize = 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));
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (OutVariableNameSize, InVariableNameSize);
Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
if (EFI_ERROR (Status)) {
@@ -358,13 +364,16 @@ RuntimeServiceGetNextVariableName (
//
// SMM comm buffer->NameSize is buffer size for return string
//
- SmmGetNextVariableName->NameSize = *VariableNameSize;
+ SmmGetNextVariableName->NameSize = OutVariableNameSize;
CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);
//
// Copy whole string
//
- CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));
+ CopyMem (SmmGetNextVariableName->Name, VariableName, InVariableNameSize);
+ if (OutVariableNameSize > InVariableNameSize) {
+ ZeroMem ((UINT8 *) SmmGetNextVariableName->Name + InVariableNameSize, OutVariableNameSize - InVariableNameSize);
+ }
//
// Send data to SMM
@@ -374,7 +383,13 @@ RuntimeServiceGetNextVariableName (
//
// Get data from SMM.
//
- *VariableNameSize = SmmGetNextVariableName->NameSize;
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // SMM CommBuffer NameSize can be a trimed value
+ // Only update VariableNameSize when needed
+ //
+ *VariableNameSize = SmmGetNextVariableName->NameSize;
+ }
if (EFI_ERROR (Status)) {
goto Done;
}
@@ -420,6 +435,7 @@ RuntimeServiceSetVariable (
EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
+ UINTN VariableNameSize;
//
// Check input parameters.
@@ -441,8 +457,9 @@ RuntimeServiceSetVariable (
//
return EFI_INVALID_PARAMETER;
}
+ VariableNameSize = StrSize (VariableName);
- if ((UINTN)(~0) - StrSize (VariableName) < OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {
+ if ((UINTN)(~0) - VariableNameSize < OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {
//
// Prevent PayloadSize overflow
//
@@ -455,7 +472,7 @@ RuntimeServiceSetVariable (
// 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_ACCESS_VARIABLE, Name) + StrSize (VariableName) + DataSize;
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + VariableNameSize + DataSize;
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_SET_VARIABLE);
if (EFI_ERROR (Status)) {
goto Done;
@@ -464,7 +481,7 @@ RuntimeServiceSetVariable (
CopyGuid ((EFI_GUID *) &SmmVariableHeader->Guid, VendorGuid);
SmmVariableHeader->DataSize = DataSize;
- SmmVariableHeader->NameSize = StrSize (VariableName);
+ SmmVariableHeader->NameSize = VariableNameSize;
SmmVariableHeader->Attributes = Attributes;
CopyMem (SmmVariableHeader->Name, VariableName, SmmVariableHeader->NameSize);
CopyMem ((UINT8 *) SmmVariableHeader->Name + SmmVariableHeader->NameSize, Data, DataSize);
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-02 12:08:58 +0000