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

[Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>]

On 12.11.2018 19:03, Heinrich Schuchardt wrote:
> On 11/12/18 7:56 AM, Simon Goldschmidt wrote:
>> On Mon, Nov 12, 2018 at 12:22 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 11/11/18 3:22 PM, Wolfgang Denk wrote:
>>>> Dear Andrea,
>>>>
>>>> In message <20181109094615.GC9586@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.
> If board_lmb_reserve() should be the solution I would prefer that not
> each individual board calls board_lmb_reserve() but that some common
> code is used to iterate over the memory reservations in the device tree.

I know that the efi loader has its own scheme of memory reservation. I 
just thought it cleaner to stay with lmb for fs_load and tftp as lmb is 
already used in image loading/booting.

But using the memory reservations from U-Boot device tree definitively 
makes sense, thanks for the hint. This should probably be done for the 
bootm code, too...

Simon

>
> Cheers
>
> Heinrich
>
>> 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