[edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
Yao, Jiewen
jiewen.yao at intel.com
Tue Sep 1 07:31:13 UTC 2020
I am sorry, that I am a little lost here.
We have discussed different patches. I am not 100% sure which one is "Laszlo's patches".
To make thing easy and record all actions, would you please reply the patch(es) you have verified, with your "tested-by:" tag?
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of wenyi,xie
> via groups.io
> Sent: Tuesday, September 1, 2020 3:11 PM
> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io; Laszlo Ersek
> <lersek at redhat.com>; Wang, Jian J <jian.j.wang at intel.com>
> Cc: songdongkuang at huawei.com; Mathews, John <john.mathews at intel.com>
> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>
> I think Laszlo's patches is OK, I have applied and tested it using my case. It can
> catch the issue.
> DEBUG code and log below,
>
> SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
> DEBUG((DEBUG_INFO, "SecDataDirEnd=0x%x.\n", SecDataDirEnd));
> for (OffSet = SecDataDir->VirtualAddress;
> OffSet < SecDataDirEnd;
> OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
> >dwLength))) {
> SecDataDirLeft = SecDataDirEnd - OffSet;
> DEBUG((DEBUG_INFO, "OffSet=0x%x, SecDataDirLeft=0x%x.\n", OffSet,
> SecDataDirLeft));
> if (SecDataDirLeft <= sizeof(WIN_CERTIFICATE)) {
> break;
> }
> WinCertificate = (WIN_CERTIFICATE *)(mImageBase + OffSet);
> DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
> ALIGN_SIZE(WinCertificate->dwLength)));
> if (SecDataDirLeft < WinCertificate->dwLength ||
> (SecDataDirLeft - WinCertificate->dwLength <
> ALIGN_SIZE(WinCertificate->dwLength))) {
> DEBUG((DEBUG_INFO, "Parameter is invalid and break.\n"));
> break;
> }
>
> SecDataDirEnd=0xFFFFFFFC.
> OffSet=0xE000, SecDataDirLeft=0xFFFF1FFC.
> WinCertificate->dwLength=0xFFFF1FFB, ALIGN_SIZE (WinCertificate-
> >dwLength)=0x5.
> Parameter is invalid and break.
> The image doesn't pass verification: VenHw(5CF32E0B-8EDF-2E44-9CDA-
> 93205E99EC1C,00000000)/VenHw(964E5B22-6459-11D2-8E39-
> 00A0C969723B,00000000)/\signed_1234_4G.efi
>
> Regards
> Wenyi
>
> On 2020/9/1 0:06, Yao, Jiewen wrote:
> > Sounds great. Appreciate your hard work on that.
> >
> > Will you post a patch to fix the issue again?
> >
> > Thank you
> > Yao Jiewen
> >
> >> -----Original Message-----
> >> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> wenyi,xie
> >> via groups.io
> >> Sent: Monday, August 31, 2020 7:24 PM
> >> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io; Laszlo
> Ersek
> >> <lersek at redhat.com>; Wang, Jian J <jian.j.wang at intel.com>
> >> Cc: songdongkuang at huawei.com; Mathews, John
> <john.mathews at intel.com>
> >> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> >> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> >>
> >> Hi，Jiewen，
> >>
> >> I modify the PE file again, this time it can pass the check in PeCoffLib and
> cause
> >> offset overflow.
> >>
> >> First, create a PE file and sign it(only one signature), then using binary edit
> tool
> >> to modify content of PE file like below,
> >> 1.check the value of SecDataDir->VirtualAddress, in my PE file, it's 0xE000
> >> 2.changing SecDataDir->Size from 0x5F8 to 0xFFFF1FFC
> >> 3.changing WinCertificate->dwLength from 0x5F1 to 0xFFFF1FFB
> >> 4.padding PE file with 0 until the size of the file is 0xFFFFFFFC(it will make the
> PE
> >> file so large)
> >> OffSet + WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength)
> is
> >> 0xE000 + 0xFFFF1FFB + 0x5 = 0x100000000
> >>
> >> Below is the DEBUG code and log, in second loop the offset overflow and
> >> become 0
> >>
> >> for (OffSet = SecDataDir->VirtualAddress;
> >> OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
> >> OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
> >>> dwLength))) {
> >> WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> >> DEBUG((DEBUG_INFO, "OffSet=0x%x.\n", OffSet));
> >> if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
> >> (WIN_CERTIFICATE) ||
> >> (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate-
> >>> dwLength) {
> >> break;
> >> }
> >> DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
> >> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
> >> ALIGN_SIZE(WinCertificate->dwLength)));
> >>
> >>
> >> SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0xFFFF1FFC.
> >> OffSet=0xE000.
> >> WinCertificate->dwLength=0xFFFF1FFB, ALIGN_SIZE (WinCertificate-
> >>> dwLength)=0x5.
> >> DxeImageVerificationLib: Image is signed but signature is not allowed by DB
> and
> >> SHA256 hash of image is notOffSet=0x0.
> >> WinCertificate->dwLength=0x5A4D, ALIGN_SIZE (WinCertificate-
> >>> dwLength)=0x3.
> >> OffSet=0x5A50.
> >> WinCertificate->dwLength=0x9024, ALIGN_SIZE (WinCertificate-
> >>> dwLength)=0x4.
> >> OffSet=0xEA78.
> >> WinCertificate->dwLength=0x0, ALIGN_SIZE (WinCertificate->dwLength)=0x0.
> >> The image doesn't pass verification: VenHw(5CF32E0B-8EDF-2E44-9CDA-
> >> 93205E99EC1C,00000000)/VenHw(964E5B22-6459-11D2-8E39-
> >> 00A0C969723B,00000000)/\signed_1234_4G.efi
> >>
> >>
> >> Regards
> >> Wenyi
> >>
> >>
> >> On 2020/8/28 14:43, Yao, Jiewen wrote:
> >>> Apology that I did not say clearly.
> >>> I mean you should not modify any code to perform an attack.
> >>>
> >>> I am not asking you to exploit the system. Attack here just means: to cause
> >> system hang or buffer overflow. That is enough.
> >>> But you cannot modify code to remove any existing checker.
> >>>
> >>> Thank you
> >>> Yao Jiewen
> >>>
> >>>> -----Original Message-----
> >>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> >> wenyi,xie
> >>>> via groups.io
> >>>> Sent: Friday, August 28, 2020 2:18 PM
> >>>> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io; Laszlo
> >> Ersek
> >>>> <lersek at redhat.com>; Wang, Jian J <jian.j.wang at intel.com>
> >>>> Cc: songdongkuang at huawei.com; Mathews, John
> >> <john.mathews at intel.com>
> >>>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> >>>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> >>>>
> >>>> Hi，Jiewen，
> >>>>
> >>>> I don't really get the meaning "create a case that bypass all checks in
> >> PeCoffLib",
> >>>> do you mean I need to create a PE file that can bypass all check in
> PeCoffLib
> >>>> without modify any
> >>>> code and then cause the problem in DxeImageVerificationLib, or just
> modify
> >>>> some code in PeCoffLib to bypass check instead of removing the calling of
> >>>> PeCoffLoaderGetImageInfo. Would
> >>>> you mind explaining a little more specifically? As far as I tried, it's really
> hard
> >> to
> >>>> reproduce the issue without touching any code.
> >>>>
> >>>> Thanks
> >>>> Wenyi
> >>>>
> >>>> On 2020/8/28 11:50, Yao, Jiewen wrote:
> >>>>> HI Wenyi
> >>>>> Thank you very much to take time to reproduce.
> >>>>>
> >>>>> I am particular interested in below:
> >>>>> "As PE file is modified, function PeCoffLoaderGetImageInfo will return
> >>>> error, so I have to remove it so that for loop can be tested in
> >>>> DxeImageVerificationHandler."
> >>>>>
> >>>>> By design, the PeCoffLib should catch illegal PE/COFF image and return
> error.
> >>>> (even it cannot catch all, it should catch most ones).
> >>>>> Other PE/COFF parser may rely on the checker in PeCoffLib and *no
> need*
> >> to
> >>>> duplicate all checkers.
> >>>>> As such, DxeImageVerificationLib (and other PeCoff consumer) just need
> >>>> checks what has passed the check in PeCoffLib.
> >>>>>
> >>>>> I don’t think you should remove the checker. If people can remove the
> >> checker,
> >>>> I am sure the rest code will be vulnerable, according to the current design.
> >>>>> Could you please to create a case that bypass all checks in PeCoffLib,
> then
> >> run
> >>>> into DxeImageVerificationLib and cause the problem? That would be more
> >>>> valuable for us.
> >>>>>
> >>>>> Thank you
> >>>>> Yao Jiewen
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> >>>> wenyi,xie
> >>>>>> via groups.io
> >>>>>> Sent: Friday, August 28, 2020 11:18 AM
> >>>>>> To: Laszlo Ersek <lersek at redhat.com>; Wang, Jian J
> >>>> <jian.j.wang at intel.com>;
> >>>>>> devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>
> >>>>>> Cc: songdongkuang at huawei.com; Mathews, John
> >>>> <john.mathews at intel.com>
> >>>>>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> >>>>>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> >>>>>>
> >>>>>> Hi，Laszlo and everyone，
> >>>>>>
> >>>>>> These days I tried to reproduce the issue，and made some progress. I
> >> think
> >>>>>> there are two way to cause overflow from a mathematical point of view,
> >>>>>> 1.As Laszlo analysed before, if WinCertificate->dwLength is large
> enough,
> >>>> close
> >>>>>> to MAX_UINT32, then (WinCertificate->dwLength + ALIGN_SIZE
> >>>> (WinCertificate-
> >>>>>>> dwLength)) will cause overflow.
> >>>>>> 2.(WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength))
> is
> >>>> good,
> >>>>>> OffSet is good, but OffSet += (WinCertificate->dwLength + ALIGN_SIZE
> >>>>>> (WinCertificate->dwLength)) cause overflow.
> >>>>>>
> >>>>>> Here I choose the second way to reproduce the issue, I choose a PE file
> >> and
> >>>> sign
> >>>>>> it with my own db certificate. Then I use binary edit tool to modify the
> PE
> >> file
> >>>> like
> >>>>>> below,
> >>>>>>
> >>>>>> 1.change SecDataDir->Size from 0x5F8 to 0xFFFF1FFF
> >>>>>> 2.change WinCertificate->dwLength from 0x5F1 to 0xFFFF1FFE
> >>>>>> SecDataDir->VirtualAddress in my PE is 0xe000 and no need to change.
> >>>>>>
> >>>>>> As PE file is modified, function PeCoffLoaderGetImageInfo will return
> error,
> >>>> so I
> >>>>>> have to remove it so that for loop can be tested in
> >>>> DxeImageVerificationHandler.
> >>>>>> The log is as below,
> >>>>>>
> >>>>>> SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0xFFFF1FFF.
> >>>>>> (First Loop)
> >>>>>> OffSet=0xE000.
> >>>>>> WinCertificate->dwLength=0xFFFF1FFE, ALIGN_SIZE (WinCertificate-
> >>>>>>> dwLength)=0x2.
> >>>>>> (Second Loop)
> >>>>>> OffSet=0x0.
> >>>>>> WinCertificate->dwLength=0x5A4D, ALIGN_SIZE (WinCertificate-
> >>>>>>> dwLength)=0x3.
> >>>>>> (Third Loop)
> >>>>>> OffSet=0x5A50.
> >>>>>> WinCertificate->dwLength=0x9024, ALIGN_SIZE (WinCertificate-
> >>>>>>> dwLength)=0x4.
> >>>>>> (Forth Loop)
> >>>>>> OffSet=0xEA78.
> >>>>>> WinCertificate->dwLength=0xAFAFAFAF, ALIGN_SIZE (WinCertificate-
> >>>>>>> dwLength)=0x1.
> >>>>>> (Fifth Loop)
> >>>>>> OffSet=0xAFB09A28.
> >>>>>>
> >>>>>> As I modify SecDataDir->Size and WinCertificate->dwLength, so in first
> >> loop
> >>>> the
> >>>>>> condition check whether the Security Data Directory has enough room
> left
> >>>> for
> >>>>>> "WinCertificate->dwLength" is ok.
> >>>>>>
> >>>>>> if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
> >>>>>> (WIN_CERTIFICATE) ||
> >>>>>> (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
> >> WinCertificate-
> >>>>>>> dwLength) {
> >>>>>> break;
> >>>>>> }
> >>>>>>
> >>>>>> In the beginning of second loop, WinCertificate->dwLength +
> ALIGN_SIZE
> >>>>>> (WinCertificate->dwLength) is 0xFFFF2000, offset is 0xE000
> >>>>>>
> >>>>>> OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
> >>>>> dwLength))
> >>>>>>
> >>>>>> Offset now is 0 and overflow happens. So even if my PE only have one
> >>>> signature,
> >>>>>> the for loop is still going untill exception happens.
> >>>>>>
> >>>>>>
> >>>>>> I can't reproduce the issue using the first way, because if WinCertificate-
> >>>>>>> dwLength is close to MAX_UINT32, it means SecDataDir->Size should
> also
> >>>> close
> >>>>>> to MAX_UINT32, or the condition check
> >>>>>> "(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
> >> WinCertificate-
> >>>>>>> dwLength" will break. But if SecDataDir->Size is very large, SecDataDir-
> >>>>>>> VirtualAddress have to be smaller than 8 bytes,
> >>>>>> because SecDataDir->VirtualAddress + SecDataDir->Size < MAX_UINT32.
> >>>>>> SecDataDir->VirtualAddress is the beginning address of the signature,
> and
> >>>> before
> >>>>>> SecDataDir->VirtualAddress is the content of origin PE file, I think it's
> >>>> impossible
> >>>>>> that the size of PE file is only 8 bytes.
> >>>>>>
> >>>>>> As I changed the one line code in DxeImageVerificationHandler to
> >> reproduce
> >>>> the
> >>>>>> issue, I'm not sure whether it's ok.
> >>>>>>
> >>>>>> Thanks
> >>>>>> Wenyi
> >>>>>>
> >>>>>> On 2020/8/19 17:26, Laszlo Ersek wrote:
> >>>>>>> On 08/18/20 17:18, Mathews, John wrote:
> >>>>>>>> I dug up the original report details. This was noted as a concern
> during a
> >>>>>> source code inspection. There was no demonstration of how it might be
> >>>>>> triggered.
> >>>>>>>>
> >>>>>>>> " There is an integer overflow vulnerability in the
> >>>>>> DxeImageVerificationHandler function when
> >>>>>>>> parsing the PE files attribute certificate table. In cases where
> >>>> WinCertificate-
> >>>>>>> dwLength is
> >>>>>>>> sufficiently large, it's possible to overflow Offset back to 0 causing an
> >>>> endless
> >>>>>> loop."
> >>>>>>>>
> >>>>>>>> The recommendation was to add stricter checking of "Offset" and the
> >>>>>> embedded length fields of certificate data
> >>>>>>>> before using them.
> >>>>>>>
> >>>>>>> Thanks for checking!
> >>>>>>>
> >>>>>>> Laszlo
> >>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>> Sent: Tuesday, August 18, 2020 1:59 AM
> >>>>>>>> To: Wang, Jian J <jian.j.wang at intel.com>; devel at edk2.groups.io;
> Yao,
> >>>>>> Jiewen <jiewen.yao at intel.com>; xiewenyi2 at huawei.com
> >>>>>>>> Cc: huangming23 at huawei.com; songdongkuang at huawei.com;
> >> Mathews,
> >>>>>> John <john.mathews at intel.com>
> >>>>>>>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> >>>>>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> >>>>>>>>
> >>>>>>>> On 08/18/20 04:10, Wang, Jian J wrote:
> >>>>>>>>> Laszlo,
> >>>>>>>>>
> >>>>>>>>> My apologies for the slow response. I'm not the original reporter but
> >>>>>>>>> just the BZ submitter. And I didn't do deep analysis to this issue.
> >>>>>>>>> The issues was reported from one internal team. Add John in loop to
> >> see
> >>>> if
> >>>>>> he knows more about it or not.
> >>>>>>>>>
> >>>>>>>>> My superficial understanding on such issue is that, if there's
> >>>>>>>>> "potential" issue in theory and hard to reproduce, it's still worthy
> >>>>>>>>> of using an alternative way to replace the original implementation
> >>>>>>>>> with no "potential" issue at all. Maybe we don't have to prove old
> way
> >> is
> >>>>>> something wrong but must prove that the new way is really safe.
> >>>>>>>>
> >>>>>>>> I agree, thanks.
> >>>>>>>>
> >>>>>>>> It would be nice to hear more from the internal team about the
> >> originally
> >>>>>> reported (even if hard-to-trigger) issue.
> >>>>>>>>
> >>>>>>>> Thanks!
> >>>>>>>> Laszlo
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Regards,
> >>>>>>>>> Jian
> >>>>>>>>>
> >>>>>>>>>> -----Original Message-----
> >>>>>>>>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf
> Of
> >>>> Laszlo
> >>>>>>>>>> Ersek
> >>>>>>>>>> Sent: Tuesday, August 18, 2020 12:53 AM
> >>>>>>>>>> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io;
> >>>>>>>>>> xiewenyi2 at huawei.com; Wang, Jian J <jian.j.wang at intel.com>
> >>>>>>>>>> Cc: huangming23 at huawei.com; songdongkuang at huawei.com
> >>>>>>>>>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> >>>>>>>>>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of
> Offset
> >>>>>>>>>>
> >>>>>>>>>> Hi Jiewen,
> >>>>>>>>>>
> >>>>>>>>>> On 08/14/20 10:53, Yao, Jiewen wrote:
> >>>>>>>>>>>> To Jiewen,
> >>>>>>>>>>>> Sorry, I don't have environment to reproduce the issue.
> >>>>>>>>>>>
> >>>>>>>>>>> Please help me understand, if you don’t have environment to
> >>>>>>>>>>> reproduce the
> >>>>>>>>>> issue, how do you guarantee that your patch does fix the problem
> and
> >>>>>>>>>> we don’t have any other vulnerabilities?
> >>>>>>>>>>
> >>>>>>>>>> The original bug report in
> >>>>>>>>>> <https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is
> >> seriously
> >>>>>>>>>> lacking. It does not go into detail about the alleged integer
> overflow.
> >>>>>>>>>> It does not quote the code, does not explain the control flow, does
> >>>>>>>>>> not identify the exact edk2 commit at which the vulnerability exists.
> >>>>>>>>>>
> >>>>>>>>>> The bug report also does not offer a reproducer.
> >>>>>>>>>>
> >>>>>>>>>> Additionally, the exact statement that the bug report does make,
> >>>>>>>>>> namely
> >>>>>>>>>>
> >>>>>>>>>> it's possible to overflow Offset back to 0 causing an endless loop
> >>>>>>>>>>
> >>>>>>>>>> is wrong (as far as I can tell anyway). It is not "OffSet" that can
> >>>>>>>>>> be overflowed to zero, but the *addend* that is added to OffSet
> can
> >>>>>>>>>> be overflowed to zero. Therefore the infinite loop will arise
> because
> >>>>>>>>>> OffSet remains stuck at its present value, and not because OffSet
> >>>>>>>>>> will be re-set to zero.
> >>>>>>>>>>
> >>>>>>>>>> For the reasons, we can only speculate as to what the actual
> problem
> >>>>>>>>>> is, unless Jian decides to join the discussion and clarifies what he
> >>>>>>>>>> had in mind originally.
> >>>>>>>>>>
> >>>>>>>>>> My understanding (or even "reconstruction") of the vulnerability is
> >>>>>>>>>> described above, and in the patches that I proposed.
> >>>>>>>>>>
> >>>>>>>>>> We can write a patch based on code analysis. It's possible to
> >>>>>>>>>> identify integer overflows based on code analysis, and it's possible
> >>>>>>>>>> to verify the correctness of fixes by code review. Obviously testing
> >>>>>>>>>> is always good, but many times, constructing reproducers for such
> >>>>>>>>>> issues that were found by code review, is difficult and time
> >>>>>>>>>> consuming. We can say that we don't fix vulnerabilities without
> >>>>>>>>>> reproducers, or we can say that we make an effort to fix them even
> if
> >>>>>>>>>> all we have is code analysis (and not a reproducer).
> >>>>>>>>>>
> >>>>>>>>>> So the above paragraph concerns "correctness". Regarding
> >>>>>>>>>> "completeness", I guarantee you that this patch does not fix *all*
> >>>>>>>>>> problems related to PE parsing. (See the other BZ tickets.) It does
> >>>>>>>>>> fix *one* issue with PE parsing. We can say that we try to fix such
> >>>>>>>>>> issues gradually (give different CVE numbers to different issues, and
> >>>>>>>>>> address them one at a time), or we can say that we rewrite PE
> parsing
> >>>>>> from the ground up.
> >>>>>>>>>> (BTW: I have seriously attempted that in the past, and I gave up,
> >>>>>>>>>> because the PE format is FUBAR.)
> >>>>>>>>>>
> >>>>>>>>>> In summary:
> >>>>>>>>>>
> >>>>>>>>>> - the problem statement is unclear,
> >>>>>>>>>>
> >>>>>>>>>> - it seems like there is indeed an integer overflow problem in the
> >>>>>>>>>> SecDataDir parsing loop, but it's uncertain whether the bug
> reporter
> >>>>>>>>>> had exactly that in mind
> >>>>>>>>>>
> >>>>>>>>>> - PE parsing is guaranteed to have other vulnerabilities elsewhere in
> >>>>>>>>>> edk2, but I'm currently unaware of other such issues in
> >>>>>>>>>> DxeImageVerificationLib specifically
> >>>>>>>>>>
> >>>>>>>>>> - even if there are other such problems (in DxeImageVerificationLib
> >>>>>>>>>> or elswehere), fixing this bug that we know about is likely
> >>>>>>>>>> worthwhile
> >>>>>>>>>>
> >>>>>>>>>> - for many such bugs, constructing a reproducer is difficult and time
> >>>>>>>>>> consuming; code analysis, and *regression-testing* are frequently
> the
> >>>>>>>>>> only tools we have. That doesn't mean we should ignore this class
> of
> >>>> bugs.
> >>>>>>>>>>
> >>>>>>>>>> (Fixing integer overflows retro-actively is more difficult than
> >>>>>>>>>> writing overflow-free code in the first place, but that ship has
> >>>>>>>>>> sailed; so we can only fight these bugs incrementally now, unless
> we
> >>>>>>>>>> can rewrite PE parsing with a new data structure from the ground
> up.
> >>>>>>>>>> Again I tried that and gave up, because the spec is not public, and
> >>>>>>>>>> what I did manage to learn about PE, showed that it was insanely
> >>>>>>>>>> over-engineered. I'm not saying that other binary / executable
> >>>>>>>>>> formats are better, of course.)
> >>>>>>>>>>
> >>>>>>>>>> Please check out my patches (inlined elsewhere in this thread), and
> >>>>>>>>>> comment whether you'd like me to post them to the list as a
> >>>>>>>>>> standalone series.
> >>>>>>>>>>
> >>>>>>>>>> Jian: it wouldn't hurt if you commented as well.
> >>>>>>>>>>
> >>>>>>>>>> Thanks
> >>>>>>>>>> Laszlo
> >>>>>>>>>>
> >>>>>>>>>>>> -----Original Message-----
> >>>>>>>>>>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf
> >> Of
> >>>>>>>>>> wenyi,xie
> >>>>>>>>>>>> via groups.io
> >>>>>>>>>>>> Sent: Friday, August 14, 2020 3:54 PM
> >>>>>>>>>>>> To: Laszlo Ersek <lersek at redhat.com>; devel at edk2.groups.io;
> Yao,
> >>>>>>>>>>>> Jiewen <jiewen.yao at intel.com>; Wang, Jian J
> >>>> <jian.j.wang at intel.com>
> >>>>>>>>>>>> Cc: huangming23 at huawei.com; songdongkuang at huawei.com
> >>>>>>>>>>>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> >>>>>>>>>>>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of
> >> Offset
> >>>>>>>>>>>>
> >>>>>>>>>>>> To Laszlo,
> >>>>>>>>>>>> Thank you for your detailed description, I agree with what you
> >>>>>>>>>>>> analyzed and
> >>>>>>>>>> I'm
> >>>>>>>>>>>> OK with your patches, it's
> >>>>>>>>>>>> correct and much simpler.
> >>>>>>>>>>>>
> >>>>>>>>>>>> To Jiewen,
> >>>>>>>>>>>> Sorry, I don't have environment to reproduce the issue.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Thanks
> >>>>>>>>>>>> Wenyi
> >>>>>>>>>>>>
> >>>>>>>>>>>> On 2020/8/14 2:50, Laszlo Ersek wrote:
> >>>>>>>>>>>>> On 08/13/20 13:55, Wenyi Xie wrote:
> >>>>>>>>>>>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> There is an integer overflow vulnerability in
> >>>>>>>>>>>>>> DxeImageVerificationHandler function when parsing the PE
> files
> >>>>>>>>>>>>>> attribute certificate table. In cases where
> >>>>>>>>>>>>>> WinCertificate->dwLength is sufficiently large, it's possible to
> >>>>>> overflow Offset back to 0 causing an endless loop.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Check offset inbetween VirtualAddress and VirtualAddress +
> Size.
> >>>>>>>>>>>>>> Using SafeintLib to do offset addition with result check.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Cc: Jiewen Yao <jiewen.yao at intel.com>
> >>>>>>>>>>>>>> Cc: Jian J Wang <jian.j.wang at intel.com>
> >>>>>>>>>>>>>> Cc: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>>>>>>>> Signed-off-by: Wenyi Xie <xiewenyi2 at huawei.com>
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>>>> ib.inf
> >>>>>>>>>> |
> >>>>>>>>>>>> 1 +
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>>>> ib.h
> >>>>>>>>>> |
> >>>>>>>>>>>> 1 +
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>>>> ib.c
> >>>>>>>>>> |
> >>>>>>>>>>>> 111 +++++++++++---------
> >>>>>>>>>>>>>> 3 files changed, 63 insertions(+), 50 deletions(-)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> diff --git
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.inf
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.inf
> >>>>>>>>>>>>>> index 1e1a639857e0..a7ac4830b3d4 100644
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.inf
> >>>>>>>>>>>>>> +++
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.inf
> >>>>>>>>>>>>>> @@ -53,6 +53,7 @@ [LibraryClasses]
> >>>>>>>>>>>>>> SecurityManagementLib
> >>>>>>>>>>>>>> PeCoffLib
> >>>>>>>>>>>>>> TpmMeasurementLib
> >>>>>>>>>>>>>> + SafeIntLib
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> [Protocols]
> >>>>>>>>>>>>>> gEfiFirmwareVolume2ProtocolGuid ##
> >>>> SOMETIMES_CONSUMES
> >>>>>>>>>>>>>> diff --git
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.h
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.h
> >>>>>>>>>>>>>> index 17955ff9774c..060273917d5d 100644
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.h
> >>>>>>>>>>>>>> +++
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.h
> >>>>>>>>>>>>>> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-
> >> Patent
> >>>>>>>>>>>>>> #include <Library/DevicePathLib.h> #include
> >>>>>>>>>>>>>> <Library/SecurityManagementLib.h> #include
> >> <Library/PeCoffLib.h>
> >>>>>>>>>>>>>> +#include <Library/SafeIntLib.h>
> >>>>>>>>>>>>>> #include <Protocol/FirmwareVolume2.h> #include
> >>>>>>>>>>>>>> <Protocol/DevicePath.h> #include <Protocol/BlockIo.h> diff -
> -
> >> git
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> index 36b87e16d53d..dbc03e28c05b 100644
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
> >>>>>>>>>> .c
> >>>>>>>>>>>>>> +++
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> @@ -1658,6 +1658,10 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> EFI_STATUS HashStatus;
> >>>>>>>>>>>>>> EFI_STATUS DbStatus;
> >>>>>>>>>>>>>> BOOLEAN IsFound;
> >>>>>>>>>>>>>> + UINT32 AlignedLength;
> >>>>>>>>>>>>>> + UINT32 Result;
> >>>>>>>>>>>>>> + EFI_STATUS AddStatus;
> >>>>>>>>>>>>>> + BOOLEAN IsAuthDataAssigned;
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> SignatureList = NULL;
> >>>>>>>>>>>>>> SignatureListSize = 0;
> >>>>>>>>>>>>>> @@ -1667,6 +1671,7 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
> >>>>>>>>>>>>>> IsVerified = FALSE;
> >>>>>>>>>>>>>> IsFound = FALSE;
> >>>>>>>>>>>>>> + Result = 0;
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> //
> >>>>>>>>>>>>>> // Check the image type and get policy setting.
> >>>>>>>>>>>>>> @@ -1850,9 +1855,10 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> // The first certificate starts at offset
> >>>>>>>>>>>>>> (SecDataDir->VirtualAddress) from
> >>>>>>>>>> the
> >>>>>>>>>>>> start of the file.
> >>>>>>>>>>>>>> //
> >>>>>>>>>>>>>> for (OffSet = SecDataDir->VirtualAddress;
> >>>>>>>>>>>>>> - OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
> >>>>>>>>>>>>>> - OffSet += (WinCertificate->dwLength + ALIGN_SIZE
> >>>>>> (WinCertificate-
> >>>>>>>>>>>>> dwLength))) {
> >>>>>>>>>>>>>> + (OffSet >= SecDataDir->VirtualAddress) && (OffSet <
> >>>>>>>>>>>>>> + (SecDataDir-
> >>>>>>>>>>>>> VirtualAddress + SecDataDir->Size));) {
> >>>>>>>>>>>>>> + IsAuthDataAssigned = FALSE;
> >>>>>>>>>>>>>> WinCertificate = (WIN_CERTIFICATE *) (mImageBase +
> OffSet);
> >>>>>>>>>>>>>> + AlignedLength = WinCertificate->dwLength + ALIGN_SIZE
> >>>>>>>>>> (WinCertificate-
> >>>>>>>>>>>>> dwLength);
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I disagree with this patch.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The primary reason for my disagreement is that the bug report
> >>>>>>>>>>>>> <https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is
> >>>>>>>>>>>>> inexact, and so this patch tries to fix the wrong thing.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> With edk2 master at commit 65904cdbb33c, it is *not* possible
> to
> >>>>>>>>>>>>> overflow the OffSet variable to zero with "WinCertificate-
> >>>>> dwLength"
> >>>>>>>>>>>>> *purely*, and cause an endless loop. Note that we have (at
> >> commit
> >>>>>>>>>>>>> 65904cdbb33c):
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> for (OffSet = SecDataDir->VirtualAddress;
> >>>>>>>>>>>>> OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
> >>>>>>>>>>>>> OffSet += (WinCertificate->dwLength + ALIGN_SIZE
> >>>>>>>>>>>>> (WinCertificate-
> >>>>>>>>>>>>> dwLength))) {
> >>>>>>>>>>>>> WinCertificate = (WIN_CERTIFICATE *) (mImageBase +
> OffSet);
> >>>>>>>>>>>>> if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
> >>>>>>>>>>>>> <= sizeof
> >>>>>>>>>>>> (WIN_CERTIFICATE) ||
> >>>>>>>>>>>>> (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
> >>>>>>>>>> WinCertificate-
> >>>>>>>>>>>>> dwLength) {
> >>>>>>>>>>>>> break;
> >>>>>>>>>>>>> }
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The last sub-condition checks whether the Security Data
> Directory
> >>>>>>>>>>>>> has enough room left for "WinCertificate->dwLength". If not,
> then
> >>>>>>>>>>>>> we break out of the loop.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> If we *do* have enough room, that is:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) >=
> >>>>>>>>>> WinCertificate-
> >>>>>>>>>>>>> dwLength
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> then we have (by adding OffSet to both sides):
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> SecDataDir->VirtualAddress + SecDataDir->Size >= OffSet +
> >>>>>>>>>>>>> WinCertificate- dwLength
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The left hand side is a known-good UINT32, and so
> incrementing
> >>>>>>>>>>>>> OffSet (a
> >>>>>>>>>>>>> UINT32) *solely* by "WinCertificate->dwLength" (also a
> UINT32)
> >>>>>>>>>>>>> does not cause an overflow.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Instead, the problem is with the alignment. The "if" statement
> >>>>>>>>>>>>> checks whether we have enough room for "dwLength", but
> then
> >>>>>>>>>>>>> "OffSet" is advanced by "dwLength" *aligned up* to the next
> >>>>>>>>>>>>> multiple of 8. And that may indeed cause various overflows.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Now, the main problem with the present patch is that it does
> not
> >>>>>>>>>>>>> fix one of those overflows. Namely, consider that "dwLength" is
> >>>>>>>>>>>>> very close to
> >>>>>>>>>>>>> MAX_UINT32 (or even think it's exactly MAX_UINT32). Then
> >> aligning
> >>>>>>>>>>>>> it up to the next multiple of 8 will yield 0. In other words,
> >>>>>> "AlignedLength"
> >>>>>>>>>>>>> will be zero.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> And when that happens, there's going to be an infinite loop just
> >>>>>>>>>>>>> the
> >>>>>>>>>>>>> same: "OffSet" will not be zero, but it will be *stuck*. The
> >>>>>>>>>>>>> SafeUint32Add() call at the bottom will succeed, but it will not
> >>>>>>>>>>>>> change the value of "OffSet".
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> More at the bottom.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
> >>>>>>>>>>>>>> <= sizeof
> >>>>>>>>>>>> (WIN_CERTIFICATE) ||
> >>>>>>>>>>>>>> (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
> >>>>>>>>>>>>>> <
> >>>>>>>>>>>> WinCertificate->dwLength) {
> >>>>>>>>>>>>>> break;
> >>>>>>>>>>>>>> @@ -1872,6 +1878,8 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> AuthData = PkcsCertData->CertData;
> >>>>>>>>>>>>>> AuthDataSize = PkcsCertData->Hdr.dwLength -
> >>>>>>>>>>>>>> sizeof(PkcsCertData-
> >>>>>>>>>>> Hdr);
> >>>>>>>>>>>>>> + IsAuthDataAssigned = TRUE;
> >>>>>>>>>>>>>> + HashStatus = HashPeImageByType (AuthData,
> AuthDataSize);
> >>>>>>>>>>>>>> } else if (WinCertificate->wCertificateType ==
> >>>>>>>>>> WIN_CERT_TYPE_EFI_GUID)
> >>>>>>>>>>>> {
> >>>>>>>>>>>>>> //
> >>>>>>>>>>>>>> // The certificate is formatted as
> >>>>>>>>>>>>>> WIN_CERTIFICATE_UEFI_GUID which
> >>>>>>>>>> is
> >>>>>>>>>>>> described in UEFI Spec.
> >>>>>>>>>>>>>> @@ -1880,72 +1888,75 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> if (WinCertUefiGuid->Hdr.dwLength <=
> >>>>>>>>>>>> OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData)) {
> >>>>>>>>>>>>>> break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> - if (!CompareGuid (&WinCertUefiGuid->CertType,
> >>>>>> &gEfiCertPkcs7Guid))
> >>>>>>>>>> {
> >>>>>>>>>>>>>> - continue;
> >>>>>>>>>>>>>> + if (CompareGuid (&WinCertUefiGuid->CertType,
> >>>>>>>>>>>>>> + &gEfiCertPkcs7Guid))
> >>>>>>>>>> {
> >>>>>>>>>>>>>> + AuthData = WinCertUefiGuid->CertData;
> >>>>>>>>>>>>>> + AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
> >>>>>>>>>>>> OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
> >>>>>>>>>>>>>> + IsAuthDataAssigned = TRUE;
> >>>>>>>>>>>>>> + HashStatus = HashPeImageByType (AuthData,
> >> AuthDataSize);
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> - AuthData = WinCertUefiGuid->CertData;
> >>>>>>>>>>>>>> - AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
> >>>>>>>>>>>> OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
> >>>>>>>>>>>>>> } else {
> >>>>>>>>>>>>>> if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE))
> {
> >>>>>>>>>>>>>> break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> - continue;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> - HashStatus = HashPeImageByType (AuthData,
> AuthDataSize);
> >>>>>>>>>>>>>> - if (EFI_ERROR (HashStatus)) {
> >>>>>>>>>>>>>> - continue;
> >>>>>>>>>>>>>> - }
> >>>>>>>>>>>>>> -
> >>>>>>>>>>>>>> - //
> >>>>>>>>>>>>>> - // Check the digital signature against the revoked
> certificate
> >> in
> >>>>>>>>>> forbidden
> >>>>>>>>>>>> database (dbx).
> >>>>>>>>>>>>>> - //
> >>>>>>>>>>>>>> - if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
> >>>>>>>>>>>>>> - Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
> >>>>>>>>>>>>>> - IsVerified = FALSE;
> >>>>>>>>>>>>>> - break;
> >>>>>>>>>>>>>> - }
> >>>>>>>>>>>>>> -
> >>>>>>>>>>>>>> - //
> >>>>>>>>>>>>>> - // Check the digital signature against the valid certificate in
> >>>>>> allowed
> >>>>>>>>>>>> database (db).
> >>>>>>>>>>>>>> - //
> >>>>>>>>>>>>>> - if (!IsVerified) {
> >>>>>>>>>>>>>> - if (IsAllowedByDb (AuthData, AuthDataSize)) {
> >>>>>>>>>>>>>> - IsVerified = TRUE;
> >>>>>>>>>>>>>> + if (IsAuthDataAssigned && !EFI_ERROR (HashStatus)) {
> >>>>>>>>>>>>>> + //
> >>>>>>>>>>>>>> + // Check the digital signature against the revoked
> >>>>>>>>>>>>>> + certificate in
> >>>>>>>>>> forbidden
> >>>>>>>>>>>> database (dbx).
> >>>>>>>>>>>>>> + //
> >>>>>>>>>>>>>> + if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
> >>>>>>>>>>>>>> + Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
> >>>>>>>>>>>>>> + IsVerified = FALSE;
> >>>>>>>>>>>>>> + break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> - }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> - //
> >>>>>>>>>>>>>> - // Check the image's hash value.
> >>>>>>>>>>>>>> - //
> >>>>>>>>>>>>>> - DbStatus = IsSignatureFoundInDatabase (
> >>>>>>>>>>>>>> - EFI_IMAGE_SECURITY_DATABASE1,
> >>>>>>>>>>>>>> - mImageDigest,
> >>>>>>>>>>>>>> - &mCertType,
> >>>>>>>>>>>>>> - mImageDigestSize,
> >>>>>>>>>>>>>> - &IsFound
> >>>>>>>>>>>>>> - );
> >>>>>>>>>>>>>> - if (EFI_ERROR (DbStatus) || IsFound) {
> >>>>>>>>>>>>>> - Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
> >>>>>>>>>>>>>> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image
> is
> >>>>>> signed
> >>>>>>>>>> but %s
> >>>>>>>>>>>> hash of image is found in DBX.\n", mHashTypeStr));
> >>>>>>>>>>>>>> - IsVerified = FALSE;
> >>>>>>>>>>>>>> - break;
> >>>>>>>>>>>>>> - }
> >>>>>>>>>>>>>> + //
> >>>>>>>>>>>>>> + // Check the digital signature against the valid
> >>>>>>>>>>>>>> + certificate in allowed
> >>>>>>>>>>>> database (db).
> >>>>>>>>>>>>>> + //
> >>>>>>>>>>>>>> + if (!IsVerified) {
> >>>>>>>>>>>>>> + if (IsAllowedByDb (AuthData, AuthDataSize)) {
> >>>>>>>>>>>>>> + IsVerified = TRUE;
> >>>>>>>>>>>>>> + }
> >>>>>>>>>>>>>> + }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> - if (!IsVerified) {
> >>>>>>>>>>>>>> + //
> >>>>>>>>>>>>>> + // Check the image's hash value.
> >>>>>>>>>>>>>> + //
> >>>>>>>>>>>>>> DbStatus = IsSignatureFoundInDatabase (
> >>>>>>>>>>>>>> - EFI_IMAGE_SECURITY_DATABASE,
> >>>>>>>>>>>>>> + EFI_IMAGE_SECURITY_DATABASE1,
> >>>>>>>>>>>>>> mImageDigest,
> >>>>>>>>>>>>>> &mCertType,
> >>>>>>>>>>>>>> mImageDigestSize,
> >>>>>>>>>>>>>> &IsFound
> >>>>>>>>>>>>>> );
> >>>>>>>>>>>>>> - if (!EFI_ERROR (DbStatus) && IsFound) {
> >>>>>>>>>>>>>> - IsVerified = TRUE;
> >>>>>>>>>>>>>> - } else {
> >>>>>>>>>>>>>> - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image
> is
> >>>>>> signed
> >>>>>>>>>> but
> >>>>>>>>>>>> signature is not allowed by DB and %s hash of image is not found
> in
> >>>>>>>>>> DB/DBX.\n",
> >>>>>>>>>>>> mHashTypeStr));
> >>>>>>>>>>>>>> + if (EFI_ERROR (DbStatus) || IsFound) {
> >>>>>>>>>>>>>> + Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
> >>>>>>>>>>>>>> + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image
> is
> >>>>>>>>>>>>>> + signed
> >>>>>>>>>>>> but %s hash of image is found in DBX.\n", mHashTypeStr));
> >>>>>>>>>>>>>> + IsVerified = FALSE;
> >>>>>>>>>>>>>> + break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>> + if (!IsVerified) {
> >>>>>>>>>>>>>> + DbStatus = IsSignatureFoundInDatabase (
> >>>>>>>>>>>>>> + EFI_IMAGE_SECURITY_DATABASE,
> >>>>>>>>>>>>>> + mImageDigest,
> >>>>>>>>>>>>>> + &mCertType,
> >>>>>>>>>>>>>> + mImageDigestSize,
> >>>>>>>>>>>>>> + &IsFound
> >>>>>>>>>>>>>> + );
> >>>>>>>>>>>>>> + if (!EFI_ERROR (DbStatus) && IsFound) {
> >>>>>>>>>>>>>> + IsVerified = TRUE;
> >>>>>>>>>>>>>> + } else {
> >>>>>>>>>>>>>> + DEBUG ((DEBUG_INFO, "DxeImageVerificationLib:
> Image
> >> is
> >>>>>>>>>>>>>> + signed
> >>>>>>>>>> but
> >>>>>>>>>>>> signature is not allowed by DB and %s hash of image is not found
> in
> >>>>>>>>>> DB/DBX.\n",
> >>>>>>>>>>>> mHashTypeStr));
> >>>>>>>>>>>>>> + }
> >>>>>>>>>>>>>> + }
> >>>>>>>>>>>>>> + }
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>> + AddStatus = SafeUint32Add (OffSet, AlignedLength,
> &Result);
> >>>>>>>>>>>>>> + if (EFI_ERROR (AddStatus)) {
> >>>>>>>>>>>>>> + break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> + OffSet = Result;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size))
> >>>>>>>>>>>>>> {
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> There are other (smaller) reasons why I dislike this patch:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> - The "IsAuthDataAssigned" variable is superfluous; we could
> use
> >>>>>>>>>>>>> the existent "AuthData" variable (with a NULL-check and a
> >>>>>>>>>>>>> NULL-assignment) similarly.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> - The patch complicates / reorganizes the control flow
> needlessly.
> >>>>>>>>>>>>> This complication originates from placing the checked "OffSet"
> >>>>>>>>>>>>> increment at the bottom of the loop, which then requires the
> >>>>>>>>>>>>> removal of all the "continue" statements. But we don't need to
> >>>>>>>>>>>>> check-and-increment at the bottom. We can keep the
> increment
> >>>>>>>>>>>>> inside the "for" statement, only extend the *existent* room
> check
> >>>>>>>>>>>>> (which I've quoted) to take the alignment into account as well.
> If
> >>>>>>>>>>>>> there is enough room for the alignment in the security data
> >>>>>>>>>>>>> directory, then that guarantees there won't be a UINT32
> overflow
> >>>>>> either.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> All in all, I'm proposing the following three patches instead. The
> >>>>>>>>>>>>> first two patches are preparation, the last patch is the fix.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Patch#1:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> From 11af0a104d34d39bf1b1aab256428ae4edbddd77 Mon
> Sep
> >> 17
> >>>>>>>>>> 00:00:00
> >>>>>>>>>>>> 2001
> >>>>>>>>>>>>>> From: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>>>>>>>> Date: Thu, 13 Aug 2020 19:11:39 +0200
> >>>>>>>>>>>>>> Subject: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib:
> >> extract
> >>>>>>>>>>>>>> SecDataDirEnd, SecDataDirLeft
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> The following two quantities:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> SecDataDir->VirtualAddress + SecDataDir->Size
> >>>>>>>>>>>>>> SecDataDir->VirtualAddress + SecDataDir->Size - OffSet
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> are used multiple times in DxeImageVerificationHandler().
> >>>>>>>>>>>>>> Introduce helper variables for them: "SecDataDirEnd" and
> >>>>>> "SecDataDirLeft", respectively.
> >>>>>>>>>>>>>> This saves us multiple calculations and significantly simplifies
> the
> >>>> code.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Note that all three summands above have type UINT32,
> >> therefore
> >>>>>>>>>>>>>> the new variables are also of type UINT32.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> This patch does not change behavior.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> (Note that the code already handles the case when the
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> SecDataDir->VirtualAddress + SecDataDir->Size
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> UINT32 addition overflows -- namely, in that case, the
> >>>>>>>>>>>>>> certificate loop is never entered, and the corruption check
> right
> >>>>>>>>>>>>>> after the loop fires.)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>>>> ib.c |
> >>>>>>>>>> 12
> >>>>>>>>>>>> ++++++++----
> >>>>>>>>>>>>>> 1 file changed, 8 insertions(+), 4 deletions(-)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> diff --git
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> index 36b87e16d53d..8761980c88aa 100644
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
> >>>>>>>>>> .c
> >>>>>>>>>>>>>> +++
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> @@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> UINT8 *AuthData;
> >>>>>>>>>>>>>> UINTN AuthDataSize;
> >>>>>>>>>>>>>> EFI_IMAGE_DATA_DIRECTORY *SecDataDir;
> >>>>>>>>>>>>>> + UINT32 SecDataDirEnd;
> >>>>>>>>>>>>>> + UINT32 SecDataDirLeft;
> >>>>>>>>>>>>>> UINT32 OffSet;
> >>>>>>>>>>>>>> CHAR16 *NameStr;
> >>>>>>>>>>>>>> RETURN_STATUS PeCoffStatus;
> >>>>>>>>>>>>>> @@ -1849,12 +1851,14 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> // "Attribute Certificate Table".
> >>>>>>>>>>>>>> // The first certificate starts at offset
> >>>>>>>>>>>>>> (SecDataDir->VirtualAddress) from
> >>>>>>>>>> the
> >>>>>>>>>>>> start of the file.
> >>>>>>>>>>>>>> //
> >>>>>>>>>>>>>> + SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir-
> >>> Size;
> >>>>>>>>>>>>>> for (OffSet = SecDataDir->VirtualAddress;
> >>>>>>>>>>>>>> - OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
> >>>>>>>>>>>>>> + OffSet < SecDataDirEnd;
> >>>>>>>>>>>>>> OffSet += (WinCertificate->dwLength + ALIGN_SIZE
> >>>>>>>>>>>>>> (WinCertificate-
> >>>>>>>>>>>>> dwLength))) {
> >>>>>>>>>>>>>> WinCertificate = (WIN_CERTIFICATE *) (mImageBase +
> OffSet);
> >>>>>>>>>>>>>> - if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
> <=
> >>>>>> sizeof
> >>>>>>>>>>>> (WIN_CERTIFICATE) ||
> >>>>>>>>>>>>>> - (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
> <
> >>>>>>>>>>>> WinCertificate->dwLength) {
> >>>>>>>>>>>>>> + SecDataDirLeft = SecDataDirEnd - OffSet;
> >>>>>>>>>>>>>> + if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
> >>>>>>>>>>>>>> + SecDataDirLeft < WinCertificate->dwLength) {
> >>>>>>>>>>>>>> break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> @@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> - if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size))
> >>>>>>>>>>>>>> {
> >>>>>>>>>>>>>> + if (OffSet != SecDataDirEnd) {
> >>>>>>>>>>>>>> //
> >>>>>>>>>>>>>> // The Size in Certificate Table or the attribute
> >>>>>>>>>>>>>> certificate table is
> >>>>>>>>>> corrupted.
> >>>>>>>>>>>>>> //
> >>>>>>>>>>>>>> --
> >>>>>>>>>>>>>> 2.19.1.3.g30247aa5d201
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Patch#2:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> From 72012c065a53582f7df695e7b9730c45f49226c6 Mon Sep
> >> 17
> >>>>>> 00:00:00
> >>>>>>>>>>>> 2001
> >>>>>>>>>>>>>> From: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>>>>>>>> Date: Thu, 13 Aug 2020 19:19:06 +0200
> >>>>>>>>>>>>>> Subject: [PATCH 2/3] SecurityPkg/DxeImageVerificationLib:
> >> assign
> >>>>>>>>>>>>>> WinCertificate after size check
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE))
> >> check
> >>>>>>>>>>>>>> only guards the de-referencing of the "WinCertificate" pointer.
> >>>>>>>>>>>>>> It does not guard the calculation of hte pointer itself:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> WinCertificate = (WIN_CERTIFICATE *) (mImageBase +
> OffSet);
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> This is wrong; if we don't know for sure that we have enough
> >> room
> >>>>>>>>>>>>>> for a WIN_CERTIFICATE, then even creating such a pointer,
> not
> >>>>>>>>>>>>>> just de-referencing it, may invoke undefined behavior.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Move the pointer calculation after the size check.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>>>> ib.c |
> >>>>>>>>>> 8
> >>>>>>>>>>>> +++++---
> >>>>>>>>>>>>>> 1 file changed, 5 insertions(+), 3 deletions(-)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> diff --git
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> index 8761980c88aa..461ed7cfb5ac 100644
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
> >>>>>>>>>> .c
> >>>>>>>>>>>>>> +++
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> @@ -1855,10 +1855,12 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> for (OffSet = SecDataDir->VirtualAddress;
> >>>>>>>>>>>>>> OffSet < SecDataDirEnd;
> >>>>>>>>>>>>>> OffSet += (WinCertificate->dwLength + ALIGN_SIZE
> >>>>>>>>>>>>>> (WinCertificate-
> >>>>>>>>>>>>> dwLength))) {
> >>>>>>>>>>>>>> - WinCertificate = (WIN_CERTIFICATE *) (mImageBase +
> >> OffSet);
> >>>>>>>>>>>>>> SecDataDirLeft = SecDataDirEnd - OffSet;
> >>>>>>>>>>>>>> - if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
> >>>>>>>>>>>>>> - SecDataDirLeft < WinCertificate->dwLength) {
> >>>>>>>>>>>>>> + if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) {
> >>>>>>>>>>>>>> + break;
> >>>>>>>>>>>>>> + }
> >>>>>>>>>>>>>> + WinCertificate = (WIN_CERTIFICATE *) (mImageBase +
> >> OffSet);
> >>>>>>>>>>>>>> + if (SecDataDirLeft < WinCertificate->dwLength) {
> >>>>>>>>>>>>>> break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> --
> >>>>>>>>>>>>>> 2.19.1.3.g30247aa5d201
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Patch#3:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> From 0bbba15b84f8f9f2cdc770a89f418aaec6cfb31e Mon Sep
> 17
> >>>>>> 00:00:00
> >>>>>>>>>>>> 2001
> >>>>>>>>>>>>>> From: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>>>>>>>> Date: Thu, 13 Aug 2020 19:34:33 +0200
> >>>>>>>>>>>>>> Subject: [PATCH 3/3] SecurityPkg/DxeImageVerificationLib:
> catch
> >>>>>>>>>> alignment
> >>>>>>>>>>>>>> overflow (CVE-2019-14562)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> The DxeImageVerificationHandler() function currently checks
> >>>>>>>>>>>>>> whether "SecDataDir" has enough room for
> >>>>>>>>>>>>>> "WinCertificate->dwLength". However,
> >>>>>>>>>>>> for
> >>>>>>>>>>>>>> advancing "OffSet", "WinCertificate->dwLength" is aligned to
> the
> >>>>>>>>>>>>>> next multiple of 8. If "WinCertificate->dwLength" is large
> >>>>>>>>>>>>>> enough, the alignment will return 0, and "OffSet" will be stuck
> at
> >>>> the
> >>>>>> same value.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Check whether "SecDataDir" has room left for both
> >>>>>>>>>>>>>> "WinCertificate->dwLength" and the alignment.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>>>> ib.c |
> >>>>>>>>>> 4
> >>>>>>>>>>>> +++-
> >>>>>>>>>>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> diff --git
> >>>>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> index 461ed7cfb5ac..e38eb981b7a0 100644
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>
> >>>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
> >>>>>>>>>> .c
> >>>>>>>>>>>>>> +++
> >>>>>>>>>>>>
> >>>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
> >>>>>>>>>>>> ib.c
> >>>>>>>>>>>>>> @@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
> >>>>>>>>>>>>>> break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>> WinCertificate = (WIN_CERTIFICATE *) (mImageBase +
> OffSet);
> >>>>>>>>>>>>>> - if (SecDataDirLeft < WinCertificate->dwLength) {
> >>>>>>>>>>>>>> + if (SecDataDirLeft < WinCertificate->dwLength ||
> >>>>>>>>>>>>>> + (SecDataDirLeft - WinCertificate->dwLength <
> >>>>>>>>>>>>>> + ALIGN_SIZE (WinCertificate->dwLength))) {
> >>>>>>>>>>>>>> break;
> >>>>>>>>>>>>>> }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> --
> >>>>>>>>>>>>>> 2.19.1.3.g30247aa5d201
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> If Wenyi and the reviewers are OK with these patches, I can
> >> submit
> >>>>>>>>>>>>> them as a standalone patch series.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Note that I do not have any reproducer for the issue; the best
> >>>>>>>>>>>>> testing that I could offer would be some light-weight Secure
> Boot
> >>>>>>>>>>>>> regression tests.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Thanks
> >>>>>>>>>>>>> Laszlo
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> .
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> .
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >>
> >>
> >
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#64875): https://edk2.groups.io/g/devel/message/64875
Mute This Topic: https://groups.io/mt/76165658/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list