The SMM Dispatcher duplicates the logic from the DXE Dispatcher to keep untrusted drivers loaded for deferred dispatching[1][2]. The DXE service Trust()[3] is supposed to be used to allow the dispatching of such images later. There is no equivalent service for SMM, and as such this is dead code that leaves untrusted images dangling in a security-critical environment. [1] https://github.com/tianocore/edk2/blob/2072c22a0d63c780b0cc6377f6d4ffb116ad6144/MdeModulePkg/Core/PiSmmCore/Dispatcher.c#L429-L432 [2] https://github.com/tianocore/edk2/blob/2072c22a0d63c780b0cc6377f6d4ffb116ad6144/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c#L473-L478 [3] https://github.com/tianocore/edk2/blob/2072c22a0d63c780b0cc6377f6d4ffb116ad6144/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c#L357
Due to the misconception that the UEFI PI specification dictates this behaviour for the DXE dispatchers, I intended to create a separate ticket for it. However, after reading it (again), I do not think this is the case. Hence, I would like to track both SMM and DXE dispatchers here. For the DXE dispatcher, the same is true as for the SMM dispatcher. Unauthenticated drivers should never be loaded, no matter what. When a driver is promoted by the DXE service Trust(), the driver should be (re-)read and loaded only then.
For the StandaloneMmCore, the loading code does not have this issue, but there is still dead code to handle the trust transition. While it is without doubt a cosmetical issue in this case, I'd like to also track it here as it's caused by the same duplicated structure.
Created attachment 789 [details] fix draft V1 for PiSmmCore and StMmCore Please find attached drafts to fix the issue in PiSmmCore, and to adapt StandaloneMmCore to remove the superfluous check. As latter is a cosmetical change, I'm fine to submit it to the list whenever you see fit, but for the time being I'll attach it here to not leave hints towards the PiSmmCore issue.
Hi, Marvin. I propose that we make this BZ public, and continue to develop your patch in the open (no CVE will be assigned). Are you OK with this approach? We discussed, and do not see this being an attack vector by itself. It would require another SMM exploit first, to achieve arbitrary execution, in order to call into these "dangling" images. Also, we were uncertain about page attributes for any such "dangling" images. Do you happen to know whether these pages would be executable or not?
Yes, public please. I don't remember whether there were any other bugs I filed you may be looking at, but feel free to make any such public. I'll report back on page attributes asap, but I am quite sure it's executable. Thank you!
Moving to confirmed and making public, per discussion in 9/1 Tianocore Infosec mtg. See comment 4.
Hey John, I did not find nor remember any explicit code to protect UEFI images in PiSmmCore. The default memory permissions are just RWX, at least for x86 [1]. The only related protection I found sets permissions based on the memory type after ReadyToLock [2]. PE data for this is gotten from the installed LoadedImage protocol [3]. As during dispatch the untrusted images are processed the same way as trusted images [4], they indeed have this protocol installed [5]. My conclusion: All images (including dangling) are RWX till ReadyToLock, and after they follow their "heuristic" (no absent, read-only, write-only, or RWX) permissions, at least for x86. [1] https://github.com/tianocore/edk2/tree/12e33dca4c0612a0975265e5ba641c6261a26455/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h#L132 [2] https://github.com/tianocore/edk2/blob/12e33dca4c0612a0975265e5ba641c6261a26455/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c#L907-L977 https://github.com/tianocore/edk2/blob/12e33dca4c0612a0975265e5ba641c6261a26455/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c#L1420-L1435 [3] https://github.com/tianocore/edk2/blob/12e33dca4c0612a0975265e5ba641c6261a26455/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c#L1262-L1313 [4] https://github.com/tianocore/edk2/blob/12e33dca4c0612a0975265e5ba641c6261a26455/MdeModulePkg/Core/PiSmmCore/Dispatcher.c#L429-L432 [5] https://github.com/tianocore/edk2/blob/12e33dca4c0612a0975265e5ba641c6261a26455/MdeModulePkg/Core/PiSmmCore/Dispatcher.c#L613-L632
