From dd5679a53a3454a5af080554aa2194662fba46ea Mon Sep 17 00:00:00 2001 From: iceman1001 Date: Sun, 26 Apr 2020 13:49:06 +0200 Subject: [PATCH] cppchecker fixes --- client/src/cmdhfmfdes.c | 114 ++++++++++++++---------- client/src/cmdhfmfu.c | 19 ++-- client/src/proxguiqt.cpp | 18 ++-- client/src/proxguiqt.h | 4 +- tools/hitag2crack/crack5/ht2crack5.c | 4 +- tools/hitag2crack/crack5gpu/ht2crack5.c | 2 +- 6 files changed, 93 insertions(+), 68 deletions(-) diff --git a/client/src/cmdhfmfdes.c b/client/src/cmdhfmfdes.c index 4b091343b..ea466c0dc 100644 --- a/client/src/cmdhfmfdes.c +++ b/client/src/cmdhfmfdes.c @@ -1478,7 +1478,7 @@ static int handler_desfire_writedata(mfdes_data_t *data, MFDES_FILE_TYPE_T type) if (data->fileno > 0x1F) return PM3_EINVARG; int datatowrite = le24toh(data->length); int offset = le24toh(data->offset); - int datasize = 0; + int datasize; int pos = 0; int recvlen = 0; int res = PM3_SUCCESS; @@ -1491,8 +1491,10 @@ static int handler_desfire_writedata(mfdes_data_t *data, MFDES_FILE_TYPE_T type) if (type == MFDES_RECORD_FILE) apdu.INS = MFDES_WRITE_RECORD; while (datatowrite > 0) { - if (datatowrite > 52) datasize = 52; - else datasize = datatowrite; + if (datatowrite > 52) + datasize = 52; + else + datasize = datatowrite; tmp[1] = offset & 0xFF; tmp[2] = (offset >> 8) & 0xFF; @@ -1690,7 +1692,7 @@ int getKeySettings(uint8_t *aid) { } // Authentication tests - int res = test_desfire_authenticate(); + res = test_desfire_authenticate(); if (res == PM3_ETIMEOUT) return res; PrintAndLogEx(SUCCESS, " [0x0A] Authenticate : %s", (res == PM3_SUCCESS) ? _YELLOW_("YES") : "NO"); @@ -1838,29 +1840,29 @@ static int CmdHF14ADesCreateApp(const char *Cmd) { swap16(fid); if (aidlength != 3) { - PrintAndLogEx(ERR, "AID must have 3 bytes length."); + PrintAndLogEx(ERR, "AID must have 3 bytes length"); return PM3_EINVARG; } if (fidlength != 2) { - PrintAndLogEx(ERR, "FID must have 2 bytes length."); - return PM3_EINVARG; - } - bool usefid = true; - if (fidlength == 0) usefid = false; - - if (keylen1 != 1) { - PrintAndLogEx(ERR, "Keysetting1 must have 1 byte length."); + PrintAndLogEx(ERR, "FID must have 2 bytes length"); return PM3_EINVARG; } + bool usefid = (fidlength != 0); + if (keylen1 != 1) { - PrintAndLogEx(ERR, "Keysetting2 must have 1 byte length."); + PrintAndLogEx(ERR, "Keysetting1 must have 1 byte length"); + return PM3_EINVARG; + } + + if (keylen2 != 1) { + PrintAndLogEx(ERR, "Keysetting2 must have 1 byte length"); return PM3_EINVARG; } if (namelen > 16) { - PrintAndLogEx(ERR, "Name has a max. of 16 bytes length."); + PrintAndLogEx(ERR, "Name has a max. of 16 bytes length"); return PM3_EINVARG; } bool usename = true; @@ -1874,7 +1876,7 @@ static int CmdHF14ADesCreateApp(const char *Cmd) { uint8_t keysetting2=0xEE;*/ if (memcmp(aid, "\x00\x00\x00", 3) == 0) { - PrintAndLogEx(WARNING, _RED_(" Creating root aid 000000 is forbidden.")); + PrintAndLogEx(WARNING, _RED_(" Creating root aid 000000 is forbidden")); return PM3_ESOFT; } @@ -1882,12 +1884,17 @@ static int CmdHF14ADesCreateApp(const char *Cmd) { memcpy(aidhdr.aid, aid, sizeof(aid)); aidhdr.keysetting1 = keysetting1; aidhdr.keysetting2 = keysetting2; + if (usefid) memcpy(aidhdr.fid, fid, sizeof(fid)); + if (usename) memcpy(aidhdr.name, name, sizeof(name)); uint8_t rootaid[3] = {0x00, 0x00, 0x00}; int res = handler_desfire_select_application(rootaid); - if (res != PM3_SUCCESS) { DropField(); return res; } + if (res != PM3_SUCCESS) { + DropField(); + return res; + } res = handler_desfire_createapp(&aidhdr, usename, usefid); DropField(); @@ -2392,6 +2399,13 @@ static int CmdHF14ADesChangeValue(const char *Cmd) { static int CmdHF14ADesWriteData(const char *Cmd) { + + uint8_t *data = (uint8_t *)calloc(0xFFFF, sizeof(uint8_t)); + if (data == NULL) { + PrintAndLogEx(ERR, "failed to allocate memory"); + return PM3_EMALLOC; + } + CLIParserInit("hf mfdes writedata", "Write data to File", "Usage:" @@ -2403,56 +2417,61 @@ static int CmdHF14ADesWriteData(const char *Cmd) { arg_strx0("aA", "aid", "", "AID for file (3 hex bytes, big endian)"), arg_strx0("nN", "fileno", "", "File Number (1 hex byte, 0x00 - 0x1F)"), arg_strx0("oO", "offset", "", "File Offset (3 hex bytes, big endian), optional"), - arg_strx0("dD", "data", "", "Data to write (hex bytes, 0xFFFFFF bytes max.)"), + arg_strx0("dD", "data", "", "Data to write (hex bytes, 0xFFFF bytes max.)"), arg_int0("type", "type", "", "File Type (0=Standard/Backup, 1=Record)"), arg_param_end }; + CLIExecWithReturn(Cmd, argtable, false); - uint8_t fileno; + int aidlength = 0; uint8_t aid[3] = {0}; CLIGetHexWithReturn(1, aid, &aidlength); + + uint8_t fileno; int filenolen = 0; CLIGetHexWithReturn(2, &fileno, &filenolen); + int offsetlength = 0; uint8_t offset[3] = {0}; CLIParamHexToBuf(arg_get_str(3), offset, 3, &offsetlength); - int dlength = 0xFFFFFF; - uint8_t *data = (uint8_t *)malloc(0xFFFFFF); - memset(data, 0x0, 0xFFFFFF); - CLIParamHexToBuf(arg_get_str(4), data, 0xFFFFFF, &dlength); + + int dlength = 0xFFFF; + CLIParamHexToBuf(arg_get_str(4), data, 0xFFFF, &dlength); + int type = arg_get_int(5); + CLIParserFree(); swap24(aid); swap24(offset); - if (type > 1) { - PrintAndLogEx(ERR, "Unknown type (0=Standard/Backup, 1=Record)."); + if (type < 0 || type > 1) { + PrintAndLogEx(ERR, "Unknown type (0=Standard/Backup, 1=Record)"); if (data) free(data); return PM3_EINVARG; } if (dlength == 0) { - PrintAndLogEx(ERR, "Data needs some hex bytes to write."); + PrintAndLogEx(ERR, "Data needs some hex bytes to write"); if (data) free(data); return PM3_EINVARG; } if (offsetlength != 3 && offsetlength != 0) { - PrintAndLogEx(ERR, "Offset needs 3 hex bytes."); + PrintAndLogEx(ERR, "Offset needs 3 hex bytes"); if (data) free(data); return PM3_EINVARG; } if (filenolen != 1) { - PrintAndLogEx(ERR, "File number is missing."); + PrintAndLogEx(ERR, "File number is missing"); if (data) free(data); return PM3_EINVARG; } if (fileno > 0x1F) { - PrintAndLogEx(ERR, "File number range is invalid (0x00-0x1F)."); + PrintAndLogEx(ERR, "File number range is invalid (0x00-0x1F)"); if (data) free(data); return PM3_EINVARG; } @@ -2466,21 +2485,23 @@ static int CmdHF14ADesWriteData(const char *Cmd) { int res = handler_desfire_select_application(aid); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Couldn't select aid."); + PrintAndLogEx(ERR, "Couldn't select aid"); DropField(); if (data) free(data); return res; } mfdes_data_t ft; + memcpy(ft.offset, offset, 3); htole24(dlength, ft.length); ft.fileno = fileno; + if (data != NULL) { ft.data = data; res = handler_desfire_writedata(&ft, type); if (res == PM3_SUCCESS) { - PrintAndLogEx(SUCCESS, "Successfully wrote data."); + PrintAndLogEx(SUCCESS, "Successfully wrote data"); } else { PrintAndLogEx(ERR, "Couldn't read data. Error %d", res); } @@ -2751,7 +2772,7 @@ static int CmdHF14ADesFormatPICC(const char *Cmd) { CLIParserFree(); if ((keylen < 8) || (keylen > 8)) { - PrintAndLogEx(ERR, "Specified key must have 8 bytes length."); + PrintAndLogEx(ERR, "Specified key must have 8 bytes length"); return PM3_EINVARG; } @@ -2766,6 +2787,7 @@ static int CmdHF14ADesFormatPICC(const char *Cmd) { payload.mode = MFDES_AUTH_PICC; payload.algo = MFDES_ALGO_DES; payload.keyno = 0; + SendCommandNG(CMD_HF_DESFIRE_AUTH1, (uint8_t *)&payload, sizeof(payload)); PacketResponseNG resp; @@ -2781,11 +2803,11 @@ static int CmdHF14ADesFormatPICC(const char *Cmd) { uint8_t flags; uint8_t datalen; uint8_t datain[FRAME_PAYLOAD_SIZE]; - } PACKED payload; - payload.datain[0] = 0xFC; - payload.flags = NONE; - payload.datalen = 1; - SendCommandNG(CMD_HF_DESFIRE_COMMAND, (uint8_t *)&payload, sizeof(payload)); + } PACKED payload_raw; + payload_raw.datain[0] = 0xFC; + payload_raw.flags = NONE; + payload_raw.datalen = 1; + SendCommandNG(CMD_HF_DESFIRE_COMMAND, (uint8_t *)&payload_raw, sizeof(payload_raw)); if (!WaitForResponseTimeout(CMD_HF_DESFIRE_COMMAND, &resp, 3000)) { PrintAndLogEx(WARNING, "Client reset command execute timeout"); DropField(); @@ -2801,7 +2823,7 @@ static int CmdHF14ADesFormatPICC(const char *Cmd) { return PM3_SUCCESS; } } else { - PrintAndLogEx(WARNING, _RED_("Auth command failed.")); + PrintAndLogEx(WARNING, _RED_("Auth command failed")); } DropField(); return PM3_SUCCESS; @@ -3124,8 +3146,10 @@ static int CmdHF14ADesDump(const char *Cmd) { uint8_t filesettings[20] = {0}; int fileset_len = 0; - int res = handler_desfire_filesettings(file_ids[j], filesettings, &fileset_len); + + res = handler_desfire_filesettings(file_ids[j], filesettings, &fileset_len); int maclen = 0; // To be implemented + if (res == PM3_SUCCESS) { //if (DecodeFileSettings(filesettings, fileset_len, maclen) != PM3_SUCCESS) { if (fileset_len == 1 + 1 + 2 + 3 + maclen) { @@ -3143,8 +3167,8 @@ static int CmdHF14ADesDump(const char *Cmd) { PrintAndLogEx(NORMAL, "\nOffset | Data | Ascii"); PrintAndLogEx(NORMAL, "----------------------------------------------------------------------------"); int len = le24toh(fdata.length); - for (int i = 0; i < len; i += 16) { - PrintAndLogEx(NORMAL, "%02d/0x%02X | %s| %s", i, i, sprint_hex(&fdata.data[i], len > 16 ? 16 : len), sprint_ascii(&fdata.data[i], len > 16 ? 16 : len)); + for (int n = 0; n < len; n += 16) { + PrintAndLogEx(NORMAL, "%02d/0x%02X | %s| %s", n, n, sprint_hex(&fdata.data[n], len > 16 ? 16 : len), sprint_ascii(&fdata.data[n], len > 16 ? 16 : len)); } free(data); } else { @@ -3161,8 +3185,8 @@ static int CmdHF14ADesDump(const char *Cmd) { if (res == PM3_SUCCESS) { PrintAndLogEx(NORMAL, "\nOffset | Value | Ascii"); PrintAndLogEx(NORMAL, "----------------------------------------------------------------------------"); - for (int i = 0; i < len; i += 16) { - PrintAndLogEx(NORMAL, "%02d/0x%02X | %s| %s", i, i, sprint_hex(&value.value[i], len > 16 ? 16 : len), sprint_ascii(&value.value[i], len > 16 ? 16 : len)); + for (int n = 0; n < len; n += 16) { + PrintAndLogEx(NORMAL, "%02d/0x%02X | %s| %s", n, n, sprint_hex(&value.value[n], len > 16 ? 16 : len), sprint_ascii(&value.value[n], len > 16 ? 16 : len)); } } else { PrintAndLogEx(ERR, "Couldn't read value. Error %d", res); @@ -3189,8 +3213,8 @@ static int CmdHF14ADesDump(const char *Cmd) { PrintAndLogEx(NORMAL, "\nOffset | Data | Ascii"); PrintAndLogEx(NORMAL, "----------------------------------------------------------------------------"); int len = le24toh(fdata.length); - for (int i = 0; i < len; i += 16) { - PrintAndLogEx(NORMAL, "%02d/0x%02X | %s| %s", i, i, sprint_hex(&fdata.data[i], len > 16 ? 16 : len), sprint_ascii(&fdata.data[i], len > 16 ? 16 : len)); + for (int n = 0; n < len; n += 16) { + PrintAndLogEx(NORMAL, "%02d/0x%02X | %s| %s", n, n, sprint_hex(&fdata.data[n], len > 16 ? 16 : len), sprint_ascii(&fdata.data[n], len > 16 ? 16 : len)); } } else { res = handler_desfire_select_application(aid); diff --git a/client/src/cmdhfmfu.c b/client/src/cmdhfmfu.c index 9bdd9b814..f70c3887e 100644 --- a/client/src/cmdhfmfu.c +++ b/client/src/cmdhfmfu.c @@ -2705,7 +2705,8 @@ static int CmdHF14AMfuOtpTearoff(const char *Cmd) { return usage_hf_mfu_otp_tearoff(); case 'b': blockNoUint = param_get8(Cmd, cmdp + 1); - if (blockNoUint < 0) { + //iceman, which blocks can be targeted? UID blocks? + if (blockNoUint < 2) { PrintAndLogEx(WARNING, "Wrong block number"); errors = true; } @@ -2713,7 +2714,7 @@ static int CmdHF14AMfuOtpTearoff(const char *Cmd) { break; case 'i': interval = param_get32ex(Cmd, cmdp + 1, interval, 10); - if (interval <= 0) { + if (interval == 0) { PrintAndLogEx(WARNING, "Wrong interval number"); errors = true; } @@ -2758,13 +2759,14 @@ static int CmdHF14AMfuOtpTearoff(const char *Cmd) { if (errors) return usage_hf_mfu_otp_tearoff(); + PrintAndLogEx(INFO, "Starting TearOff test - Selected Block no: %u", blockNoUint); + + uint32_t actualTime = startTime; - printf("\nStarting TearOff test - Selected Block no: %d ...\n", blockNoUint); while (actualTime <= (timeLimit - interval)) { - printf("\nTrying attack at: %d us\n", actualTime); - printf("\n.....\n"); - printf("\nReading block before attack: \n"); + PrintAndLogEx(INFO, "Using tear-off at: %" PRIu32 " us", actualTime); + PrintAndLogEx(INFO, "Reading block BEFORE attack"); clearCommandBuffer(); SendCommandOLD(CMD_HF_MIFAREU_READBL, blockNoUint, 0, 0, NULL, 0); @@ -2780,7 +2782,7 @@ static int CmdHF14AMfuOtpTearoff(const char *Cmd) { } } - printf("\n.....\n"); + PrintAndLogEx(INFO, "....."); clearCommandBuffer(); SendCommandOLD(CMD_HF_MFU_OTP_TEAROFF, blockNoUint, actualTime, 0, teardata, 8); @@ -2789,8 +2791,7 @@ static int CmdHF14AMfuOtpTearoff(const char *Cmd) { return PM3_ESOFT; } - - printf("\nReading block after attack: \n"); + PrintAndLogEx(INFO, "Reading block AFTER attack"); clearCommandBuffer(); SendCommandOLD(CMD_HF_MIFAREU_READBL, blockNoUint, 0, 0, NULL, 0); diff --git a/client/src/proxguiqt.cpp b/client/src/proxguiqt.cpp index f2fb5986a..855b41c87 100644 --- a/client/src/proxguiqt.cpp +++ b/client/src/proxguiqt.cpp @@ -349,11 +349,11 @@ void Plot::setMaxAndStart(int *buffer, size_t len, QRect plotRect) { GraphStart = startMax; } if (GraphStart > len) return; - int vMin = INT_MAX, vMax = INT_MIN, v = 0; + int vMin = INT_MAX, vMax = INT_MIN; uint32_t sample_index = GraphStart ; for (; sample_index < len && xCoordOf(sample_index, plotRect) < plotRect.right() ; sample_index++) { - v = buffer[sample_index]; + int v = buffer[sample_index]; if (v < vMin) vMin = v; if (v > vMax) vMax = v; } @@ -381,7 +381,7 @@ void Plot::PlotDemod(uint8_t *buffer, size_t len, QRect plotRect, QRect annotati first_delta_x += BitStart * PlotGridX; if (BitStart > (int)len) return; int delta_x = 0; - int v = 0; +// int v = 0; //printf("first_delta_x %i, grid_delta_x %i, DemodStart %i, BitStart %i\n",first_delta_x,grid_delta_x,DemodStart, BitStart); painter->setPen(getColor(graphNum)); @@ -393,9 +393,9 @@ void Plot::PlotDemod(uint8_t *buffer, size_t len, QRect plotRect, QRect annotati delta_x = 0; int clk = first_delta_x; for (int i = BitStart; i < (int)len && xCoordOf(delta_x + DemodStart, plotRect) < plotRect.right(); i++) { - for (int ii = 0; ii < (clk) && i < (int)len && xCoordOf(DemodStart + delta_x + ii, plotRect) < plotRect.right() ; ii++) { - x = xCoordOf(DemodStart + delta_x + ii, plotRect); - v = buffer[i] * 200 - 100; + for (int j = 0; j < (clk) && i < (int)len && xCoordOf(DemodStart + delta_x + j, plotRect) < plotRect.right() ; j++) { + x = xCoordOf(DemodStart + delta_x + j, plotRect); + int v = buffer[i] * 200 - 100; y = yCoordOf(v, plotRect, absVMax); @@ -405,7 +405,7 @@ void Plot::PlotDemod(uint8_t *buffer, size_t len, QRect plotRect, QRect annotati QRect f(QPoint(x - 3, y - 3), QPoint(x + 3, y + 3)); painter->fillRect(f, QColor(100, 255, 100)); } - if (ii == (int)clk / 2) { + if (j == (int)clk / 2) { //print label sprintf(str, "%u", buffer[i]); painter->drawText(x - 8, y + ((buffer[i] > 0) ? 18 : -6), str); @@ -482,7 +482,7 @@ void Plot::PlotGraph(int *buffer, size_t len, QRect plotRect, QRect annotationRe //Graph annotations painter->drawPath(penPath); char str[200]; - sprintf(str, "max=%d min=%d mean=%d n=%d/%zu CursorAVal=[%d] CursorBVal=[%d]", + sprintf(str, "max=%d min=%d mean=%d n=%u/%zu CursorAVal=[%d] CursorBVal=[%d]", vMax, vMin, vMean, i, len, buffer[CursorAPos], buffer[CursorBPos]); painter->drawText(20, annotationRect.bottom() - 23 - 20 * graphNum, str); @@ -595,7 +595,7 @@ void Plot::paintEvent(QPaintEvent *event) { //Draw annotations char str[200]; - sprintf(str, "@%d dt=%d [%2.2f] zoom=%2.2f CursorAPos=%d CursorBPos=%d GridX=%d GridY=%d (%s) GridXoffset=%d", + sprintf(str, "@%u dt=%u [%2.2f] zoom=%2.2f CursorAPos=%u CursorBPos=%u GridX=%d GridY=%d (%s) GridXoffset=%d", GraphStart, CursorBPos - CursorAPos, ((int32_t)(CursorBPos - CursorAPos)) / CursorScaleFactor, diff --git a/client/src/proxguiqt.h b/client/src/proxguiqt.h index 70a9cc6c1..7f9a82126 100644 --- a/client/src/proxguiqt.h +++ b/client/src/proxguiqt.h @@ -35,8 +35,8 @@ class Plot: public QWidget { double GraphPixelsPerPoint; // How many visual pixels are between each sample point (x axis) uint32_t CursorAPos; uint32_t CursorBPos; - void PlotGraph(int *buffer, size_t len, QRect r, QRect r2, QPainter *painter, int graphNum); - void PlotDemod(uint8_t *buffer, size_t len, QRect r, QRect r2, QPainter *painter, int graphNum, uint32_t plotOffset); + void PlotGraph(int *buffer, size_t len, QRect plotRect, QRect annotationRect, QPainter *painter, int graphNum); + void PlotDemod(uint8_t *buffer, size_t len, QRect plotRect, QRect annotationRect, QPainter *painter, int graphNum, uint32_t plotOffset); void plotGridLines(QPainter *painter, QRect r); int xCoordOf(int i, QRect r); int yCoordOf(int v, QRect r, int maxVal); diff --git a/tools/hitag2crack/crack5/ht2crack5.c b/tools/hitag2crack/crack5/ht2crack5.c index 42afbde88..33145d99a 100644 --- a/tools/hitag2crack/crack5/ht2crack5.c +++ b/tools/hitag2crack/crack5/ht2crack5.c @@ -162,7 +162,7 @@ int main(int argc, char *argv[]) { for (size_t i0 = 0; i0 < 1 << 20; i0++) { uint64_t state0 = expand(0x5806b4a2d16c, i0); - if (f(state0) == target >> 31) { + if (f(state0) == (target >> 31)) { candidates[layer_0_found++] = state0; } } @@ -185,7 +185,7 @@ void *find_state(void *thread_d) { for (size_t index = thread; index < layer_0_found; index += thread_count) { if (((index / thread_count) & 0xFF) == 0) - printf("Thread %lu slice %lu/%lu\n", thread, index / thread_count / 256 + 1, layer_0_found / thread_count / 256); + printf("Thread %zu slice %zu/%zu\n", thread, index / thread_count / 256 + 1, layer_0_found / thread_count / 256); uint64_t state0 = candidates[index]; bitslice(state0 >> 2, &state[0], 46, false); for (size_t bit = 0; bit < 8; bit++) { diff --git a/tools/hitag2crack/crack5gpu/ht2crack5.c b/tools/hitag2crack/crack5gpu/ht2crack5.c index 37df9df65..8e6c29c76 100644 --- a/tools/hitag2crack/crack5gpu/ht2crack5.c +++ b/tools/hitag2crack/crack5gpu/ht2crack5.c @@ -231,7 +231,7 @@ int main(int argc, char *argv[]) { for (size_t i0 = 0; i0 < 1 << 20; i0++) { uint64_t state0 = expand(0x5806b4a2d16c, i0); - if (f(state0) == target >> 31) { + if (f(state0) == (target >> 31)) { // cf kernel, state is now split in 3 shorts >> 2 candidates[(layer_0_found * 3) + 0] = (uint16_t)((state0 >> (32 + 2)) & 0xffff); candidates[(layer_0_found * 3) + 1] = (uint16_t)((state0 >> (16 + 2)) & 0xffff);