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

[Heinrich Schuchardt]

On 11/12/18 7:56 AM, Simon Goldschmidt wrote:
> 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.

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.

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
>