From 180bc14d31a28d80b19d1528728538efb01c7598 Mon Sep 17 00:00:00 2001 From: Cole Munz Date: Mon, 27 Jul 2026 01:15:22 -0500 Subject: [PATCH] Fix heap out-of-bounds reads on short iCLASS dump files hf iclass eload -v, hf iclass decrypt and hf iclass eview -v all cast a loaded dump buffer to picopass_hdr_t* and print it without checking the buffer is that big. pm3_load_dump() sizes the allocation to the real file content and only clamps when it's larger than the max, never pads a short file up, so a truncated dump gives a short heap block and the print code reads past it. Same root cause as #3412, which only covered view. decrypt is the worst of the three: no -v needed, and it probes fixed offsets through block 9 - app_issuer_area, the aa1_encryption flag and block 7 inside iclass_decode_credentials(), then its own block 9 PACS/PIN check - regardless of what applimit or decryptedlen/8 claim the real block count is. Check the loaded length against what's about to be read. eload and eview skip only the unsafe verbose print so a legitimately short custom dump still uploads; decrypt refuses below 10 blocks because the credential decode always reaches block 9. Signed-off-by: Cole Munz --- client/src/cmdhficlass.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/client/src/cmdhficlass.c b/client/src/cmdhficlass.c index 23149bf70f..c073be5eef 100644 --- a/client/src/cmdhficlass.c +++ b/client/src/cmdhficlass.c @@ -1937,8 +1937,12 @@ static int CmdHFiClassELoad(const char *Cmd) { } if (verbose) { - print_picopass_header((picopass_hdr_t *) dump); - print_picopass_info((picopass_hdr_t *) dump); + if (bytes_read < sizeof(picopass_hdr_t)) { + PrintAndLogEx(FAILED, "Error, dump file is too small to be a valid iCLASS dump - bytes: %zu, expected at least: %zu", bytes_read, sizeof(picopass_hdr_t)); + } else { + print_picopass_header((picopass_hdr_t *) dump); + print_picopass_info((picopass_hdr_t *) dump); + } } PrintAndLogEx(NORMAL, ""); @@ -2059,8 +2063,12 @@ static int CmdHFiClassEView(const char *Cmd) { } if (verbose) { - print_picopass_header((picopass_hdr_t *) dump); - print_picopass_info((picopass_hdr_t *) dump); + if (bytes < sizeof(picopass_hdr_t)) { + PrintAndLogEx(FAILED, "Error, only %u bytes downloaded, too small to be a valid iCLASS dump - expected at least: %zu", bytes, sizeof(picopass_hdr_t)); + } else { + print_picopass_header((picopass_hdr_t *) dump); + print_picopass_info((picopass_hdr_t *) dump); + } } PrintAndLogEx(NORMAL, ""); @@ -2326,6 +2334,18 @@ static int CmdHFiClassDecrypt(const char *Cmd) { return res; } + // Below (and in iclass_decode_credentials()) we unconditionally probe + // fixed offsets up through block 9 -- app_issuer_area (block 5), the + // aa1_encryption flag and block 7 in iclass_decode_credentials(), and + // the block 9 PACS/PIN check here -- regardless of what applimit or + // decryptedlen/8 say the "real" block count is. All 10 blocks (0-9) + // must actually be present in the loaded file before any of that runs. + if (decryptedlen < 10 * PICOPASS_BLOCK_SIZE) { + PrintAndLogEx(FAILED, "Error, dump file is too small - bytes: %zu, expected at least: %d", decryptedlen, 10 * PICOPASS_BLOCK_SIZE); + free(decrypted); + return PM3_EFILE; + } + have_file = true; }