Re: [PATCH] crypto: asymmetric_keys - fix OOB read in pefile_parse_binary

On Thu, Aug 13, 2026 at 5:57 PM Fabrice Derepas
<[email protected]> wrote:
>
> pefile_parse_binary() reads the size field of the certificate table's
> data-directory entry, which sits at fixed index 4 of the PE optional
> header's data directory:
>
>         ctx->certs_size = ddir->certs.size;
>
> but nothing ensures index 4 is present. n_data_dirents (the untrusted
> NumberOfRvaAndSizes) is only upper-bounded against header_size and may be
> 0, and header_size need only satisfy cursor < header_size < datalen. A
> crafted PE with n_data_dirents = 0 and a tiny header_size therefore causes
> the ddir->certs.size read to land past the end of the image (CWE-125). The
> chkaddr() that bounds the certificate blob runs only after this read.
>
> verify_pefile_signature() is reached from kexec_file_load() (the
> lockdown/secure-boot enforced PE-image signature path), and the image is
> parsed before its signature is checked. The trigger needs CAP_SYS_BOOT and
> the access is out-of-bounds read only (no write).
>
> Require the certificate table's data-directory entry (index 4) to be
> present; the existing upper-bound check then keeps ddir->certs within
> [cursor, header_size).
>
> Fixes: 26d1164be37f ("pefile: Parse a PE binary to find a key and a signature contained therein")
> Assisted-by: copilot-cli:claude-opus-4-6 frama-c
> Signed-off-by: Fabrice Derepas <[email protected]>
> ---
> Reproduced under KASAN (CONFIG_KASAN_GENERIC, x86-64) with a KUnit case that

Any reason not to commit this as well?

> builds a crafted PE (valid MZ/PE/PE32 magics, data_dirs = 0, header_size just
> past the optional header) and calls verify_pefile_signature() with a NULL
> keyring -- the parse runs before any signature check. On an unpatched kernel:
>
>     BUG: KASAN: slab-out-of-bounds in verify_pefile_signature+0x1d6/0x950
>     Read of size 4 ... in pefile_parse_binary() (inlined)
>
> With this patch the crafted image is rejected (-ELIBBAD) before the read and
> the case passes with no KASAN report. The test is not included here; happy to
> submit it separately.
>
>  crypto/asymmetric_keys/verify_pefile.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
> index cec99db..e7f8bdb 100644
> --- a/crypto/asymmetric_keys/verify_pefile.c
> +++ b/crypto/asymmetric_keys/verify_pefile.c
> @@ -87,6 +87,10 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
>         if (ctx->n_data_dirents > (ctx->header_size - cursor) / sizeof(*dde))
>                 return -ELIBBAD;
>
> +       /* The certificate table entry is at fixed index 4 of the data directory. */
> +       if (ctx->n_data_dirents <= 4)

My agent suggested replacing 4 with offsetof(struct data_directory,
certs) / sizeof(*dde) to make it more obvious, however not married to
it.

> +               return -ELIBBAD;
> +
>         ddir = pebuf + cursor;
>         cursor += sizeof(*dde) * ctx->n_data_dirents;
>
> base-commit: 3d6d817622b0a9721e3cc404df3469171582be13
> --
> 2.53.0
>
>

Ignat