Re: [PATCH] crypto: asymmetric_keys - bound section data range in pefile_digest_pe_contents

On Thu, Aug 13, 2026 at 7:26 PM Fabrice Derepas
<[email protected]> wrote:
>
> pefile_digest_pe_contents() hashes each PE section with
>
>         crypto_shash_update(desc, pebuf + ctx->secs[i].data_addr,
>                             ctx->secs[i].raw_data_size);
>
> where both data_addr and raw_data_size come straight from the untrusted PE
> section table. pefile_parse_binary() bounds only the section table's
> location, never the sections' data ranges, and nothing else on the path
> checks them. A section with data_addr and/or raw_data_size pointing outside
> the image therefore reads past the pelen-byte buffer (CWE-125); a large
> raw_data_size makes it read far past the end and fault.
>
> Commit f7dd32c5179d ("crypto: asymmetric_keys - fix OOB read in
> pefile_digest_pe_contents") recently fixed a sibling underflow in this same
> function (the hashed_bytes + certs_size trailer computation); the
> per-section read below was left unchecked.
>
> This loop runs only after verify_pkcs7_signature() succeeds, so it requires
> a validly signed image -- but not an attacker signing key. The PKCS#7
> signature covers the SpcIndirectDataContent (a self-contained digest in the
> certificate table), not the live section bytes, so tampering data_addr in a
> publicly available signed image leaves the signature valid at that step
> while the out-of-bounds read fires when this function recomputes the
> digest. It is thus reachable by a local CAP_SYS_BOOT user via
> kexec_file_load() with a tampered signed image; the access is an
> out-of-bounds read only, but a large raw_data_size faults, which is a
> denial of service when panic_on_oops is set.
>
> Reject a section whose data range does not lie within the image before
> hashing it.
>
> Fixes: af316fc442ef ("pefile: Digest the PE binary and compare to the PKCS#7 data")
> Assisted-by: copilot-cli:claude-opus-4-6 frama-c
> Signed-off-by: Fabrice Derepas <[email protected]>
> ---
> Note: f7dd32c5179d, which fixed the sibling underflow in this function, was
> assigned CVE-2026-64544; this closes the per-section read it did not cover.
>
> Reproduced under KASAN (CONFIG_KASAN_GENERIC, x86-64). Two things were shown:
>
>   1. The defect + fix: a KUnit case calls pefile_digest_pe_contents() with a
>      crafted context whose section has data_addr = pelen. Unpatched:
>      slab-out-of-bounds read; patched: -ELIBBAD before the hash.
>
>   2. The reachability, end-to-end against a real trusted keyring: a real EFI
>      binary was signed with a test key embedded via CONFIG_SYSTEM_TRUSTED_KEYS,
>      then its section data_addr was tampered (leaving the PKCS#7 byte-identical).
>      Results from verify_pefile_signature(): the untampered image verifies (0),
>      one signed by an untrusted key is rejected at verify_pkcs7_signature()
>      (-ENOKEY), and the tampered-but-trusted image passes that step and takes
>      the OOB read in pefile_digest_pe_contents(). So no signing key is required.
>
> The KUnit case (item 1) is easy to package and I can submit it separately; the
> end-to-end setup (item 2) needs an embedded trusted key and signed images, so
> it is harder to ship standalone. Neither is included here.
>
>  crypto/asymmetric_keys/verify_pefile.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
> index cec99db..4e84fcf 100644
> --- a/crypto/asymmetric_keys/verify_pefile.c
> +++ b/crypto/asymmetric_keys/verify_pefile.c
> @@ -292,6 +292,11 @@ static int pefile_digest_pe_contents(const void *pebuf, unsigned int pelen,
>                 i = canon[loop];
>                 if (ctx->secs[i].raw_data_size == 0)
>                         continue;
> +               if (ctx->secs[i].data_addr > pelen ||
> +                   ctx->secs[i].raw_data_size > pelen - ctx->secs[i].data_addr) {

Can we do this validation earlier? Perhaps in pefile_parse_binary()?
The idea is that if the data is malformed we should probably not start
hashing at all. You could also reuse chaddr() macro for the check.

> +                       kfree(canon);
> +                       return -ELIBBAD;
> +               }
>                 ret = crypto_shash_update(desc,
>                                           pebuf + ctx->secs[i].data_addr,
>                                           ctx->secs[i].raw_data_size);
> base-commit: 3d6d817622b0a9721e3cc404df3469171582be13
> --
> 2.53.0
>
>

Ignat