Re: [Bug 203761] New: efivar_ssdt_iter is subject to stack corruption when the input name_size is 0

Re: [Bug 203761] New: efivar_ssdt_iter is subject to stack corruption when the input name_size is 0

[Ard Biesheuvel]

On Thu, 30 May 2019 at 18:04, <bugzilla-daemon@bugzilla.kernel.org> wrote:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=203761
>
>             Bug ID: 203761
>            Summary: efivar_ssdt_iter is subject to stack corruption when
>                     the input name_size is 0
>            Product: EFI
>            Version: unspecified
>     Kernel Version: 4.18.0-20-generic
>           Hardware: Intel
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: high
>           Priority: P1
>          Component: Boot
>           Assignee: efi@kernel-bugs.osdl.org
>           Reporter: steven.yutang@gmail.com
>         Regression: No
>
> Reported also at https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1830951
>
> function efivar_ssdt_iter in efi.c has the following code:
>
>         char utf8_name[EFIVAR_SSDT_NAME_MAX];
>         int limit = min_t(unsigned long, EFIVAR_SSDT_NAME_MAX, name_size);
>         ucs2_as_utf8(utf8_name, name, limit - 1);
>
> In this short snippet of code, we can see typical issues due to unsigned long
> <-> int casting.
>
> 1. mismatch of signedness.
> 2. loss of precision.
>
> The input of name_size is signed long, gets compared against an unsigned long
> of a fixed size, then stored as a signed int (this is mostly okay because of
> the known max size), but it then gets passed to a function takes unsigned long
> without checking the range.
>
> Here, the input name_size is 0, limit also is 0, but limit - 1 = -1, and then
> casts to ULONGMAX to ucs2_as_utf8 and corrupts the stack storage with a size of
> only EFIVAR_SSDT_NAME_MAX.
>

Re: [Bug 203761] New: efivar_ssdt_iter is subject to stack corruption when the input name_size is 0

[Matthew Garrett]

On Fri, May 31, 2019 at 2:03 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> > The input of name_size is signed long, gets compared against an unsigned long
> > of a fixed size, then stored as a signed int (this is mostly okay because of
> > the known max size), but it then gets passed to a function takes unsigned long
> > without checking the range.
> >
> > Here, the input name_size is 0, limit also is 0, but limit - 1 = -1, and then
> > casts to ULONGMAX to ucs2_as_utf8 and corrupts the stack storage with a size of
> > only EFIVAR_SSDT_NAME_MAX.

This is a legitimate bug, but anyone in a position to trigger this is
also in a position to inject an arbitrary SSDT which then means
arbitrary code execution in the kernel, so I don't think there's any
security-relevant impact.