On 10/19/18 08:40, Gao, Liming wrote: > Laszlo: > I try to answer your question. I also include the BZ submitter > brent.holtsc...@intel.com. Holtsclaw, please add your comments if my > info is not enough. > > Thanks > Liming >> -----Original Message----- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Friday, October 19, 2018 12:01 AM >> To: Gao, Liming <liming....@intel.com>; Zeng, Star >> <star.z...@intel.com>; edk2-devel@lists.01.org; Ard Biesheuvel >> <ard.biesheu...@linaro.org> >> Subject: Re: [edk2] [Patch 0/3] Add more checker for Tianocompress >> and Ueficompress >> >> On 10/18/18 15:36, Gao, Liming wrote: >>> Laszlo and Star: >>> Thank your notes. I will add CVE number in patch subject although >>> it will make subject long than 80 characters. >> >> I agree the subject will be overlong, but I also think that including >> the CVE numbers is important enough for that. >> >>> Here is my proposed patch subject: CVE-2017-5731..5735 MdePkg: Add >>> more checker in UefiDecompressLib to access the valid buffer only. >> >> I suggest (based on tradition) that we keep the normal subject at the >> front, and then we append the CVE numbers at the end. Also, we should >> spell out all those CVE identifiers individually, if the same patch >> solves them all. It should be possible to search the subject line for >> any one of these CVE numbers in separation, using the official CVE >> number format. >> > > So, your proposal is like: MdePkg: Add more checker in > UefiDecompressLib to access the valid buffer only CVE-2017-5731 > CVE-2017-5732 CVE-2017-5733 CVE-2017-5734 CVE-2017-5735
Yes: MdePkg: Add more checker in UefiDecompressLib to access the valid buffer only (CVE-2017-5731 CVE-2017-5732 CVE-2017-5733 CVE-2017-5734 CVE-2017-5735) It looks terrible, but the real subject is still readable to the left, and subjects with searchable CVE numbers take priority (in my opinion anyway). Actually: I wonder why we needed five different CVEs, if they can all be fixed with a small, single patch. More precisely: looking at the patch in more detail, I see that the patch fixes multiple functions / separate buffer overflows. Is it possible to associate each CVE with a specific, small code change in the patch? Because if it is possible, then I think we should split the patch *per CVE*. The subjects would go: - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5731) - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5732) - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5733) - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5734) - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5735) (71 characters, in each subject) If such separation is technically possible, then I think it would be an improvement; minimally for documentation purposes. >>> In PEI phase, the recovery image is from the external device. If >>> the recovery image has the corrupt EFI compression section, they >>> will be handled by EFI Decompression PPI. >> >> In the PEI phase, if the recovery image is crafted, it could cause a >> buffer overflow during decompression. However, if the recovery image >> is crafted, it might as well decompress cleanly, and once it is >> dispatched, do "bad things". Do the decompression and the dispatch >> occur at different privilege levels? >> > > This patch focuses on the wrong decompression data that cause the > decompression failure or hang. The data content can be signed and > verified. > >>> In DXE phase, UEFI option ROM is the third party code. If it is EFI >>> compression option ROM, EFI decompression protocol will be used to >>> decode its data. I don't think SMM uses EFI decompression protocol. >>> UefiDecompressionLib is used as EFI compression PPI/Protocol. It >>> matches PI EFI compression section instead of GUID section. So, it >>> has no GUID extraction PPI/Protocol. >> >> In the DXE phase, if the option ROM is crafted, it could cause a >> buffer overflow when it is decompressed. But, again, how is that >> different from when a crafted oprom decompresses cleanly, and then >> does "bad things" when it is dispatched? >> >> Here (in the DXE phase), I can imagine two answers myself: >> >> (1) Decompression occurs before Secure Boot validation, but dispatch >> occurs only after. Therefore a crafted UEFI image could cause >> problems via decompression even if it would fail SB verification >> later. >> >> (2) Decompression of UEFI option ROMs occurs before PlatformBDS locks >> down SMRAM and lockboxes. However, the execution of UEFI option ROMs >> is deferred until after the lockdown. >> >> Do these scenarios apply? Because, if they do, I agree the issue >> qualifies as privilege escalation. >> > > Yes. Decompression happen early. After decompression, PE image will be > verified. Got it now. Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
