From 0c55190f40bff9d286f51d42b0275324497f7461 Mon Sep 17 00:00:00 2001 From: czhang46 Date: Fri, 12 Apr 2013 05:59:11 +0000 Subject: Update SMM variable DXE driver GetNextVariable interface to comply with UEFI spec VariableNameSize is the returned buffer size. GetNextVariable should behavior correct if it is bigger than SMM communication buffer or less than string size of VariableName. Signed-off-by: Chao Zhang Reviewed-by : Dong Guo Reviewed-by : Fu Siyuan Reviewed-by : Zeng Star git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@14258 6f19259b-4bc3-4df7-8a09-765794883524 --- .../RuntimeDxe/VariableSmmRuntimeDxe.c | 38 +++++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) (limited to 'SecurityPkg') diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c index 103a129..f1111b3 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 -- cgit v1.1