Re: [PATCH] crypto: asymmetric_keys: verify_pefile: fix integer underflow in pefile_strip_sig_wrapper

Re: [PATCH] crypto: asymmetric_keys: verify_pefile: fix integer underflow in pefile_strip_sig_wrapper

[sashiko-bot]

> The pefile_strip_sig_wrapper() function validates the win_certificate
> wrapper that precedes the PKCS#7 signature in a PE binary.  It checks
> that the wrapper length does not exceed the certificate table size
> (``wrapper.length > ctx->sig_len``) but fails to verify that the
> wrapper length is at least ``sizeof(struct win_certificate)`` (8 bytes).
> 
> When a crafted PE image supplies a wrapper with ``length`` in the range
> [1, 7], the subsequent arithmetic::
> 
>     ctx->sig_len = wrapper.length;      /* e.g. 1 */
>     ctx->sig_offset += sizeof(wrapper);
>     ctx->sig_len -= sizeof(wrapper);    /* 1 - 8 = 0xFFFFFFF9 (unsigned) */
> 
> underflows the unsigned ``ctx->sig_len`` to a value near UINT_MAX.  The
> following ``sig_len < 4`` sanity check is bypassed because 0xFFFFFFF9 is
> [ ... ]
> prevent the underflow.
> 
> Fixes: 09dacbbda935 ("pefile: Strip the wrapper off of the cert data block")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gu Bowen <gubowen5@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110015657.528337-1-gubowen5@huawei.com?part=1

[PATCH] crypto: asymmetric_keys: verify_pefile: fix integer underflow in pefile_strip_sig_wrapper

[Gu Bowen]

The pefile_strip_sig_wrapper() function validates the win_certificate
wrapper that precedes the PKCS#7 signature in a PE binary.  It checks
that the wrapper length does not exceed the certificate table size
(``wrapper.length > ctx->sig_len``) but fails to verify that the
wrapper length is at least ``sizeof(struct win_certificate)`` (8 bytes).

When a crafted PE image supplies a wrapper with ``length`` in the range
[1, 7], the subsequent arithmetic::

    ctx->sig_len = wrapper.length;      /* e.g. 1 */
    ctx->sig_offset += sizeof(wrapper);
    ctx->sig_len -= sizeof(wrapper);    /* 1 - 8 = 0xFFFFFFF9 (unsigned) */

underflows the unsigned ``ctx->sig_len`` to a value near UINT_MAX.  The
following ``sig_len < 4`` sanity check is bypassed because 0xFFFFFFF9 is
greater than 4.

The underflowed ``sig_len`` is then passed to verify_pkcs7_signature()
and onwards to asn1_ber_decoder().  The ASN.1 decoder's own
``datalen > 65535`` entry guard can be satisfied by constructing a
PKCS#7 length field of exactly 65535 (e.g. ``30 82 ff fb``), allowing
the decoder to proceed and attempt to read OID octets beyond the
allocated vmalloc buffer, triggering a page fault in look_up_OID()::

  BUG: unable to handle page fault for address: ffffd1e986cb2000
  #PF: supervisor read access in kernel mode
  RIP: 0010:look_up_OID+0x14/0x100
  Call Trace:
   pkcs7_note_OID+0x33/0xa0
   asn1_ber_decoder+0x363/0xbd0
   pkcs7_parse_message+0xd9/0x1b0
   verify_pkcs7_signature+0x24/0x80
   verify_pefile_signature+0x7b/0xf0
   kexec_kernel_verify_pe_sig+0x1b/0x40
   kimage_file_alloc_init+0x1d8/0x3f0
   __se_sys_kexec_file_load+0xc6/0x270

This is reachable through the kexec_file_load syscall on architectures
that verify PE signatures (x86 bzImage with
CONFIG_KEXEC_BZIMAGE_VERIFY_SIG, arm64 Image with
CONFIG_KEXEC_IMAGE_VERIFY_SIG), requiring CAP_SYS_BOOT.

The oops occurs while the caller holds kexec_lock; if the faulting task
is killed, the lock is never released, leaving all subsequent kexec and
kdump operations permanently returning -EBUSY until reboot.

Add a lower-bound check on wrapper.length before the subtraction to
prevent the underflow.

Fixes: 09dacbbda935 ("pefile: Strip the wrapper off of the cert data block")
Cc: stable@vger.kernel.org
Signed-off-by: Gu Bowen <gubowen5@huawei.com>
---
 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 cec99db14129..cfa85ccb1cf7 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -146,6 +146,11 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
 			ctx->sig_len, wrapper.length);
 		return -ELIBBAD;
 	}
+	if (wrapper.length < sizeof(wrapper)) {
+		pr_warn("Signature wrapper too small (%x < %zx)\n",
+			wrapper.length, sizeof(wrapper));
+		return -ELIBBAD;
+	}
 	if (wrapper.revision != WIN_CERT_REVISION_2_0) {
 		pr_warn("Signature is not revision 2.0\n");
 		return -ENOTSUPP;
-- 
2.43.0