On Sun, May 10, 2020 at 02:36:51PM -0600, Simon Glass wrote: > Hi Patrick, > > On Fri, 8 May 2020 at 12:56, Patrick Wildt <[email protected]> wrote: > > > > Hi, > > > > even though this mail has a diff, it's not supposed to be a patch. I > > have started adjusting my fuzzer to the upstreamed EFI Secure Boot code > > and I hit a few issues right away. Two of those I have already sent and > > were reviewed. I have seen more, but since I needed to reschedule some > > things I couldn't continue and unfortunately have to postpone fuzzing > > the code. I can assure you if someone starts fuzzing that code, we will > > find a few more bugs. > > > > Especially since this is "Secure Boot", this part should definitely be > > air tight. > > > > One thing I saw is that the virt_size is smaller then the memcpy below > > at the "Copy PE headers" line then actually copies. > > > > Another thing I saw is SizeOfRawData can be bigger then the VirtualSize, > > and PointerToRawData + SizeOfRawData bigger then the allocated size. > > > > I'm not sure if this is the right place to do the checks. Maybe they > > need to be at the place where we are adding the image regions. I guess > > the fields should be properly verified in view of "does it make sense?". > > > > Also I'm questioning whether it's a good idea to continue parsing the > > file if it's already clear that the signature can't be verified against > > the "db". I can understand that you'd like to finish loading the file > > into an object, and some other instance decides whether or not it's fine > > to start that image, but you also open yourself to issues when you're > > parsing a file that obviously is against the current security policy. > > > > If you keep on parsing a file that obviously (because tested against the > > "db") cannot be allowed to boot anyway, the checks for the headers need > > to be even tighter. > > > > I'd be unhappy to see U-Boot CVEs come up regarding PE parsing issues. > > > > Best regards, > > Patrick > > Can I suggest adding some more unit tests? > > Regards, > Simon
Hi, It's a good suggestion. Now it only needs someone with enough time to actually do it. :-) I'll come back to this in a few weeks when my current project becomes quieter. I wish I could right now. Regards, Patrick
