[PATCH v2] crypto: asymmetric_keys - bound PE section ranges in pefile_parse_binary
Fabrice Derepas <[email protected]>
| Newsgroups | org.kernel.vger.linux-crypto,org.infradead.lists.kexec,org.kernel.vger.keyrings,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 the section table's location
but not 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 was left unchecked.
The digest 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 the digest is
recomputed. 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.
Validate each section's data range against the image when the section
table is parsed, reusing the existing chkaddr() bounds macro. A malformed
range is rejected with -ELIBBAD at parse time -- before signature
verification and before any hashing -- so a bad image is never digested.
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]>
---
v2:
- move the check into pefile_parse_binary() and reuse chkaddr(), per
Ignat Korchagin's review [1], so a malformed image is rejected at parse
time -- before signature verification and before hashing. v1 put the
check in the pefile_digest_pe_contents() section loop.
- because chkaddr() runs on every section, v2 also validates zero-size
(raw_data_size == 0) sections, which v1 and the shipping digest loop
skip. This only rejects a zero-size section whose data_addr points past
the image -- malformed input that real PEs never produce (.bss uses
PointerToRawData = 0, which passes) -- and matches the "don't start on
malformed data" intent.
[1] https://lore.kernel.org/all/CAOs+rJXZ++7_o4L6LuT567FjdYsa=6NEki3qcFSRX+ehJxpoVg@mail.gmail.com/
Tested under KASAN (CONFIG_KASAN_GENERIC, x86-64) with a KUnit case that
calls verify_pefile_signature() on real images: a signed-then-tampered
image (a section's data_addr set to pelen) is now rejected with -ELIBBAD in
pefile_parse_binary(), before verify_pkcs7_signature() and with no KASAN
splat, while an untampered signed image passes parse and reaches the
signature check (-ENOKEY, as the test key is not trusted in this build).
v1's KASAN repro -- a crafted context taken straight into the digest loop --
showed the slab-out-of-bounds this prevents.
crypto/asymmetric_keys/verify_pefile.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index cec99db14..c447597f7 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -30,6 +30,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
const struct data_dirent *dde;
const struct section_header *sec;
size_t cursor, datalen = pelen;
+ unsigned int loop;
kenter("");
@@ -112,6 +113,14 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
return -ELIBBAD;
ctx->secs = pebuf + cursor;
+ /* pefile_digest_pe_contents() hashes each section's raw data using
+ * these fields directly; reject a section whose data range falls
+ * outside the image so hashing never reads past the buffer.
+ */
+ for (loop = 0; loop < ctx->n_sections; loop++)
+ chkaddr(0, ctx->secs[loop].data_addr,
+ ctx->secs[loop].raw_data_size);
+
return 0;
}
base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
--
2.53.0