From 6341f40b4cc46da46bdc071b71dd7bbc74a66de3 Mon Sep 17 00:00:00 2001 From: iceman1001 Date: Sat, 12 Sep 2026 10:44:00 +0200 Subject: [PATCH] hf mf hardnested: fix the nonce reply length, 9 bytes per pair not 4 per nonce Thanks @TheArchitect0880 for pointing it out and suggested a first fix. MifareAcquireEncryptedNonces packs two 4 byte encrypted nonces plus one byte holding both their encrypted parity nibbles into every entry, but the reply declared num_nonces * 4 bytes. The client walks that buffer 9 bytes at a time, so on RDV4 it read 612 bytes out of a 544 byte payload and handed roughly 15 of every 136 nonces to add_nonce() from stale packet buffer content. Those fake nonces went into the .bin nonce file too. Count pairs instead of bytes. num_nonces now reports whole pairs only, so a button abort or a static nonce bailout part way through a pair drops the dangling nonce rather than shipping a half built entry whose parity nibble was never filled in. MFC_NONCE_PAIR_SIZE and MFC_MAX_NONCE_PAIRS document the layout next to mf_nonces_resp_t so the 9 vs 4 confusion cannot come back, and the client now refuses a reply too short for the nonce count it carries. Both acquisition functions collect straight into the reply buffer rather than a second PM3_CMD_DATA_SIZE stack array, which halves the stack used per call. MifareAcquireNonces also returned isOK = 2 on button press, which is not a PM3_* status; that is PM3_EOPABORTED now. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 +++--- armsrc/mifarecmd.c | 47 +++++++++++++++++++++++++--------------- client/src/cmdhfmfhard.c | 15 ++++++++++--- include/pm3_cmd.h | 8 +++++++ 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f192a65a2..92a8d833d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,10 @@ All notable changes to this project will be documented in this file. This project uses the changelog in accordance with [keepchangelog](http://keepachangelog.com/). Please use this to write notable changes, which is not the same as git commit log... ## [unreleased][unreleased] -- Changed `hf mfdes detect` - with no `-n` it now sweeps every key number the application declares instead of only key 0; the first hit pins the algo for the rest (@iceman1001) -- Fixed `hf mfdes detect` - the card error counter was shared by every key type, so a bad run during the DES pass could abort the AES pass before it tried a single key (@iceman1001) -- Fixed `hf mfdes detect` - with no `-f` it drew candidates from the MIFARE Plus 16 byte key list,it now defaults to the bundled `mfdes_default_keys` dictionary (@iceman1001) +- Fixed `hf mf hardnested` - the device declared `num_nonces * 4` bytes but packs 9 bytes per nonce pair. Thanks @TheArchitect0880 (@iceman1001) +- Changed `hf mfdes detect` - with no `-n` it now sweeps every key number the application declares instead of only key 0 (@iceman1001) +- Fixed `hf mfdes detect` - the card error counter was shared by every key type, so a bad run during the DES pass could abort the AES pass (@iceman1001) +- Fixed `hf mfdes detect` - with no `-f` it drew candidates from the MIFARE Plus 16 byte key list, it now defaults to the bundled `mfdes_default_keys` dictionary (@iceman1001) - Fixed `hf mfdes detect` - LRP was only found when the key settings happened to be unreadable (@iceman1001) - Added `hf mfdes chk --schann` - now detect the secure channel EV/EV2/LRP mode; detected per application and can be overridden (@iceman1001) - Fixed `hf mfdes chk` - found keys were tracked per key number instead of per application, so a key number recovered on one AID was skipped later (@iceman1001) diff --git a/armsrc/mifarecmd.c b/armsrc/mifarecmd.c index c35953ac0..d803ce08e 100644 --- a/armsrc/mifarecmd.c +++ b/armsrc/mifarecmd.c @@ -991,9 +991,14 @@ void MifareAcquireNonces(const mf_acquire_nonces_t *payload) { uint8_t uid[10] = {0x00}; uint8_t answer[MAX_MIFARE_FRAME_SIZE] = {0x00}; uint8_t par[1] = {0x00}; - uint8_t buf[PM3_CMD_DATA_SIZE] = {0x00}; + + // collect straight into the reply buffer, no second copy on the stack + uint8_t respbuf[sizeof(mf_nonces_resp_t) + (MFC_MAX_NONCES * 4)] = {0x00}; + mf_nonces_resp_t *response = (mf_nonces_resp_t *)respbuf; + uint8_t *buf = response->nonces; + uint32_t cuid = 0; - int16_t isOK = 0; + int16_t isOK = PM3_SUCCESS; uint16_t num_nonces = 0; uint8_t cascade_levels = 0; uint8_t blockNo = payload->blockno; @@ -1019,7 +1024,7 @@ void MifareAcquireNonces(const mf_acquire_nonces_t *payload) { // Test if the action was cancelled if (BUTTON_PRESS()) { - isOK = 2; + isOK = PM3_EOPABORTED; field_off = true; break; } @@ -1072,12 +1077,12 @@ void MifareAcquireNonces(const mf_acquire_nonces_t *payload) { LED_C_OFF(); LED_B_ON(); - uint8_t respbuf[PM3_CMD_DATA_SIZE] = {0x00}; - mf_nonces_resp_t *response = (mf_nonces_resp_t *)respbuf; + + // one bare 4 byte nonce per entry here, unlike the paired layout used by + // MifareAcquireEncryptedNonces. The loop above already caps num_nonces + uint16_t noncelen = num_nonces * 4; response->cuid = cuid; response->num_nonces = num_nonces; - uint16_t noncelen = MIN((uint16_t)(num_nonces * 4), (uint16_t)(MFC_MAX_NONCES * 4)); - memcpy(response->nonces, buf, noncelen); reply_ng(CMD_HF_MIFARE_ACQ_NONCES, isOK, respbuf, sizeof(mf_nonces_resp_t) + noncelen); LED_B_OFF(); @@ -1106,12 +1111,17 @@ void MifareAcquireEncryptedNonces(const mf_acquire_nonces_t *payload) { uint8_t uid[10] = {0x00}; uint8_t receivedAnswer[MAX_MIFARE_FRAME_SIZE] = {0x00}; uint8_t par_enc[1] = {0x00}; - uint8_t buf[PM3_CMD_DATA_SIZE] = {0x00}; + + // collect straight into the reply buffer, no second copy on the stack + uint8_t respbuf[sizeof(mf_nonces_resp_t) + (MFC_MAX_NONCE_PAIRS * MFC_NONCE_PAIR_SIZE)] = {0x00}; + mf_nonces_resp_t *response = (mf_nonces_resp_t *)respbuf; + uint8_t *buf = response->nonces; uint64_t ui64Key = bytes_to_num(payload->key, 6); uint32_t cuid = 0; int16_t isOK = PM3_SUCCESS; uint16_t num_nonces = 0; + uint16_t num_pairs = 0; uint8_t nt_par_enc = 0; uint8_t cascade_levels = 0; uint8_t blockNo = payload->blockno; @@ -1139,7 +1149,7 @@ void MifareAcquireEncryptedNonces(const mf_acquire_nonces_t *payload) { uint8_t prev_enc_nt[] = {0, 0, 0, 0}; uint8_t prev_counter = 0; - for (uint16_t i = 0; i <= (MFC_MAX_NONCES * 4) - 9;) { + while (num_pairs < MFC_MAX_NONCE_PAIRS) { // Test if the action was cancelled if (BUTTON_PRESS()) { @@ -1196,14 +1206,15 @@ void MifareAcquireEncryptedNonces(const mf_acquire_nonces_t *payload) { } num_nonces++; + uint16_t ofs = num_pairs * MFC_NONCE_PAIR_SIZE; if (num_nonces % 2) { - memcpy(buf + i, receivedAnswer, 4); + memcpy(buf + ofs, receivedAnswer, 4); nt_par_enc = par_enc[0] & 0xf0; } else { nt_par_enc |= par_enc[0] >> 4; - memcpy(buf + i + 4, receivedAnswer, 4); - memcpy(buf + i + 8, &nt_par_enc, 1); - i += 9; + memcpy(buf + ofs + 4, receivedAnswer, 4); + buf[ofs + 8] = nt_par_enc; + num_pairs++; } @@ -1231,12 +1242,12 @@ void MifareAcquireEncryptedNonces(const mf_acquire_nonces_t *payload) { LED_C_OFF(); crypto1_deinit(pcs); LED_B_ON(); - uint8_t respbuf[PM3_CMD_DATA_SIZE] = {0x00}; - mf_nonces_resp_t *response = (mf_nonces_resp_t *)respbuf; + + // only whole pairs are transferred. An abort or a static nonce bailout can + // leave a dangling first nonce, it has no parity nibble yet so it is dropped + uint16_t noncelen = num_pairs * MFC_NONCE_PAIR_SIZE; response->cuid = cuid; - response->num_nonces = num_nonces; - uint16_t noncelen = MIN((uint16_t)(num_nonces * 4), (uint16_t)(MFC_MAX_NONCES * 4)); - memcpy(response->nonces, buf, noncelen); + response->num_nonces = num_pairs * 2; reply_ng(CMD_HF_MIFARE_ACQ_ENCRYPTED_NONCES, isOK, respbuf, sizeof(mf_nonces_resp_t) + noncelen); LED_B_OFF(); diff --git a/client/src/cmdhfmfhard.c b/client/src/cmdhfmfhard.c index 3f3f47b1b..88204437f 100644 --- a/client/src/cmdhfmfhard.c +++ b/client/src/cmdhfmfhard.c @@ -1733,9 +1733,18 @@ static int acquire_nonces(uint8_t blockNo, uint8_t keyType, uint8_t *key, uint8_ if (initialize == false) { const mf_nonces_resp_t *nresp = (const mf_nonces_resp_t *)resp.data.asBytes; - uint16_t num_sampled_nonces = nresp->num_nonces; + uint16_t num_sampled_nonces = nresp->num_nonces & ~1; // nonces come in pairs const uint8_t *bufp = nresp->nonces; + if (resp.length < sizeof(mf_nonces_resp_t) + ((num_sampled_nonces / 2) * MFC_NONCE_PAIR_SIZE)) { + PrintAndLogEx(FAILED, "Truncated nonce reply, got %u bytes for %u nonces", resp.length, num_sampled_nonces); + if (nonce_file_write) { + fclose(fnonces); + } + DropField(); + return PM3_ESOFT; + } + for (uint16_t i = 0; i < num_sampled_nonces; i += 2) { uint32_t nt_enc1 = bytes_to_num(bufp, 4); uint32_t nt_enc2 = bytes_to_num(bufp + 4, 4); @@ -1760,10 +1769,10 @@ static int acquire_nonces(uint8_t blockNo, uint8_t keyType, uint8_t *key, uint8_ num_acquired_nonces += add_res; if (nonce_file_write) { - fwrite(bufp, 1, 9, fnonces); + fwrite(bufp, 1, MFC_NONCE_PAIR_SIZE, fnonces); fflush(fnonces); } - bufp += 9; + bufp += MFC_NONCE_PAIR_SIZE; } //total_num_nonces += num_sampled_nonces; diff --git a/include/pm3_cmd.h b/include/pm3_cmd.h index 56f643c35..c0268adcd 100644 --- a/include/pm3_cmd.h +++ b/include/pm3_cmd.h @@ -541,6 +541,14 @@ typedef struct { // most 4 byte nonces that fit alongside the reply header in one frame #define MFC_MAX_NONCES ((PM3_CMD_DATA_SIZE - sizeof(mf_nonces_resp_t)) / 4) +// ACQ_ENCRYPTED_NONCES does not store bare nonces. It stores pairs: two 4 byte +// encrypted nonces followed by one byte holding both their encrypted parity +// nibbles, high nibble first. num_nonces in the reply counts nonces, so the +// payload is always (num_nonces / 2) * MFC_NONCE_PAIR_SIZE bytes and +// num_nonces is always even. +#define MFC_NONCE_PAIR_SIZE 9 +#define MFC_MAX_NONCE_PAIRS ((PM3_CMD_DATA_SIZE - sizeof(mf_nonces_resp_t)) / MFC_NONCE_PAIR_SIZE) + // CMD_HF_MIFARE_CHKKEYS_FAST payload. // Replaces three bit-packed oldargs: // arg0 = sectorcnt | firstchunk<<8 | lastchunk<<12 | singlesector_params<<16