[U-Boot] CVE-2018-18439, CVE-2018-18440 - U-Boot verified boot bypass vulnerabilities

[Simon Goldschmidt]

On Mon, Nov 12, 2018 at 12:22 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 11/11/18 3:22 PM, Wolfgang Denk wrote:
> > Dear Andrea,
> >
> > In message <20181109094615.GC9586 at lambda.inversepath.com> you wrote:
> >>
> >> Exactly, merely checking RAM size is not sufficient. The specific memory
> >> layout would need to be accounted for which means understanding where the
> >> stack and heap are located, their direction of growth and to ensure that the
> >> loaded payload can never overwrite them along with all other U-Boot data
> >> segments.
> >
> > This is pretty easy.  On all architectures I'm aware of the stack
> > has the lowest location in memory, and is growing downward.
> >
> >> This is not easy given that the stack and heap size I think can only be
> >> guessed and not precisely limited, additionally board configurations have the
> >> ability to set arbitrary stack, relocation and load addresses which
> >> complicates things even further in understanding exactly how the memory
> >> layout is set.
> >
> > I think this is not that complicated.  At least in standard U-Boot
> > (not speaking for SPL) it should be sufficient to check the current
> > stack pointer (which is easy to read) and take this a upper limit of
> > available/allowed memory. If we add some reasonable safety margin
> > (say, 1 MB or so) we should be really safe.
>
> Unfortunately this does not hold true. E.g. the Odroid-C2 has the secure
> monitor in the middle of the RAM. You would not want to overwrite those
> addresses.
>
> For a board with a device tree all reserved memory areas should be
> secured against overwriting.

That's why I proposed to use the already existing memory reservation
scheme 'lmb' (used in loading boot images).

In your case, 'board_lmb_reserve' should make sure the secure monitor
does not get overwritten.
The 'arch_lmb_reserve' function for arm already ensures U-Boot text,
heap and stack don't get overwritten. It could be improved to reserve
+1M to the current stack pointer where it does reserve +4K now.

I am working on a patch for the 'load' issue (which could be reused
for the tftp issue). There are some problems with the existing lmb
code though, which delayed me a bit. However, given that this doesn't
make it into the 2018.11 release, anyway, I figured some more days to
get it cleaner won't hurt...

Simon

>
> Best regards
>
> Heinrich
>
> >
> >>> Additionally, your patch checks the loaded file's size without taking
> >>> the load address into account. So unless I read that wrong, your check
> >>> is only valid for 'addr == 0'.
> >
> > The approach is also not appliccable to networ boot; with TFTP we
> > don't know the image size in advance.
> >
> > Eventyally the boundary checking should be done where the image
> > content actually gets copied to memory.
> >
> > Best regards,
> >
> > Wolfgang Denk
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot