From fd9501f582c8ffa10e9ed70f4aca2f66fe0a3931 Mon Sep 17 00:00:00 2001 From: "Doug Cook (WINDOWS)" Date: Sat, 30 Nov 2024 17:13:43 -0800 Subject: DxeRngLib: GetRandomNumber spurious success The GetRandomNumber functions in DxeRngLib can return success without actually generating a random number. This occurs because there are code paths through `GenerateRandomNumberViaNist800Algorithm` that do not initialize the `Status` variable. - Assume mFirstAlgo == MAX_UINTN (no secure algorithms available) - Assume none of the secure algorithms have `Available` set. - Assume PcdEnforceSecureRngAlgorithms is TRUE. In this condition, the `Status` variable is never initialized, `Buffer` data is never touched. It is fairly likely that Status is 0, so we can return EFI_SUCCESS without writing anything to Buffer. Fix is to set `Status = error_code` in this code path. `EFI_SECURITY_VIOLATION` seems appropriate. Signed-off-by: Doug Cook --- MdePkg/Library/DxeRngLib/DxeRngLib.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c b/MdePkg/Library/DxeRngLib/DxeRngLib.c index 4c1b5721ea..fcd489aabd 100644 --- a/MdePkg/Library/DxeRngLib/DxeRngLib.c +++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c @@ -204,7 +204,10 @@ GenerateRandomNumberViaNist800Algorithm ( } } - if (!PcdGetBool (PcdEnforceSecureRngAlgorithms)) { + if (PcdGetBool (PcdEnforceSecureRngAlgorithms)) { + // Platform does not permit the use of the default (insecure) algorithm. + Status = EFI_SECURITY_VIOLATION; + } else { // If all the other methods have failed, use the default method from the RngProtocol Status = mRngProtocol->GetRNG (mRngProtocol, NULL, BufferSize, Buffer); DEBUG ((DEBUG_INFO, "%a: GetRNG algorithm default - Status = %r\n", __func__, Status)); -- cgit