Skip to contentSkip to conversation

crypto: asymmetric_keys: verify_pefile: fix integer underflow in pefile_strip_sig_wrapper

PATCH New Gu Bowen 2 participants 2 messages Nov 10

Subsystems (6)
lore
Share & tools
lore

Gu Bowen

author
#0 lore raw

PATCH 1/1 crypto: asymmetric_keys: verify_pefile: fix integer underflow in pefile_strip_sig_wrapper

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")
---
 crypto/asymmetric_keys/verify_pefile.c | 5 +++++
 1 file changed, 5 insertions(+)
crypto/asymmetric_keys/verify_pefile.c +5 -0
index cec99db14129..cfa85ccb1cf7 100644
@@ -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;
Signature · 1 line
2.43.0
#1 lore raw
Sashiko has reviewed this patch and found no issues.
Quoted 13 lines

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) */
Quoted 8 lines

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")

Sashiko has reviewed this patch and found no issues.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1