From: Martin Holst Swende Date: Mon, 15 Feb 2016 17:43:25 +0000 (+0100) Subject: Merge pull request #162 from marshmellow42/CoverityFixes X-Git-Tag: v2.3.0~7 X-Git-Url: http://git.zerfleddert.de/cgi-bin/gitweb.cgi/proxmark3-svn/commitdiff_plain/b31ef4f5104384d0d301dcd27d11b3a9861b1d82?hp=b5cefff12c4b8c415926a1ccef3f3f528629e2d0 Merge pull request #162 from marshmellow42/CoverityFixes Coverity fixes - mainly from @iceman1001 s fork --- diff --git a/armsrc/iclass.c b/armsrc/iclass.c index 4e4854ca..f99d0eca 100644 --- a/armsrc/iclass.c +++ b/armsrc/iclass.c @@ -1447,7 +1447,7 @@ static void TransmitIClassCommand(const uint8_t *cmd, int len, int *samples, int } WDT_HIT(); } - if (samples) *samples = (c + *wait) << 3; + if (samples && wait) *samples = (c + *wait) << 3; } diff --git a/client/cmdhficlass.c b/client/cmdhficlass.c index 12a7141e..309880d2 100644 --- a/client/cmdhficlass.c +++ b/client/cmdhficlass.c @@ -33,8 +33,6 @@ #include "usb_cmd.h" #include "cmdhfmfu.h" -#define llX PRIx64 - static int CmdHelp(const char *Cmd); #define ICLASS_KEYS_MAX 8 @@ -283,8 +281,13 @@ int CmdHFiClassELoad(const char *Cmd) { long fsize = ftell(f); fseek(f, 0, SEEK_SET); - uint8_t *dump = malloc(fsize); + if (fsize < 0) { + PrintAndLog("Error, when getting filesize"); + fclose(f); + return 1; + } + uint8_t *dump = malloc(fsize); size_t bytes_read = fread(dump, 1, fsize, f); fclose(f); @@ -368,10 +371,13 @@ int CmdHFiClassDecrypt(const char *Cmd) { //Open the tagdump-file FILE *f; char filename[FILE_PATH_SIZE]; - if(opt == 'f' && param_getstr(Cmd, 1, filename) > 0) - { + if(opt == 'f' && param_getstr(Cmd, 1, filename) > 0) { f = fopen(filename, "rb"); - }else{ + if ( f == NULL ) { + PrintAndLog("Could not find file %s", filename); + return 1; + } + } else { return usage_hf_iclass_decrypt(); } @@ -591,7 +597,7 @@ int CmdHFiClassReader_Dump(const char *Cmd) { errors = param_gethex(tempStr, 0, CreditKEY, dataLen); } else if (dataLen == 1) { keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr <= ICLASS_KEYS_MAX) { + if (keyNbr < ICLASS_KEYS_MAX) { memcpy(CreditKEY, iClass_Key_Table[keyNbr], 8); } else { PrintAndLog("\nERROR: Credit KeyNbr is invalid\n"); @@ -625,7 +631,7 @@ int CmdHFiClassReader_Dump(const char *Cmd) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr <= ICLASS_KEYS_MAX) { + if (keyNbr < ICLASS_KEYS_MAX) { memcpy(KEY, iClass_Key_Table[keyNbr], 8); } else { PrintAndLog("\nERROR: Credit KeyNbr is invalid\n"); @@ -884,7 +890,7 @@ int CmdHFiClass_WriteBlock(const char *Cmd) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr <= ICLASS_KEYS_MAX) { + if (keyNbr < ICLASS_KEYS_MAX) { memcpy(KEY, iClass_Key_Table[keyNbr], 8); } else { PrintAndLog("\nERROR: Credit KeyNbr is invalid\n"); @@ -933,7 +939,7 @@ int usage_hf_iclass_clone(void) { } int CmdHFiClassCloneTag(const char *Cmd) { - char filename[FILE_PATH_SIZE]; + char filename[FILE_PATH_SIZE] = {0}; char tempStr[50]={0}; uint8_t KEY[8]={0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}; uint8_t keyNbr = 0; @@ -987,7 +993,7 @@ int CmdHFiClassCloneTag(const char *Cmd) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr <= ICLASS_KEYS_MAX) { + if (keyNbr < ICLASS_KEYS_MAX) { memcpy(KEY, iClass_Key_Table[keyNbr], 8); } else { PrintAndLog("\nERROR: Credit KeyNbr is invalid\n"); @@ -1038,6 +1044,7 @@ int CmdHFiClassCloneTag(const char *Cmd) { if (startblock<5) { PrintAndLog("You cannot write key blocks this way. yet... make your start block > 4"); + fclose(f); return 0; } // now read data from the file from block 6 --- 19 @@ -1046,7 +1053,11 @@ int CmdHFiClassCloneTag(const char *Cmd) { // else we have to create a share memory int i; fseek(f,startblock*8,SEEK_SET); - fread(tag_data,sizeof(iclass_block_t),endblock - startblock + 1,f); + if ( fread(tag_data,sizeof(iclass_block_t),endblock - startblock + 1,f) == 0 ) { + PrintAndLog("File reading error."); + fclose(f); + return 2; + } uint8_t MAC[4]={0x00,0x00,0x00,0x00}; uint8_t div_key[8]={0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}; @@ -1168,7 +1179,7 @@ int CmdHFiClass_ReadBlock(const char *Cmd) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr <= ICLASS_KEYS_MAX) { + if (keyNbr < ICLASS_KEYS_MAX) { memcpy(KEY, iClass_Key_Table[keyNbr], 8); } else { PrintAndLog("\nERROR: Credit KeyNbr is invalid\n"); @@ -1305,8 +1316,13 @@ int CmdHFiClassReadTagFile(const char *Cmd) { long fsize = ftell(f); fseek(f, 0, SEEK_SET); - uint8_t *dump = malloc(fsize); + if ( fsize < 0 ) { + PrintAndLog("Error, when getting filesize"); + fclose(f); + return 1; + } + uint8_t *dump = malloc(fsize); size_t bytes_read = fread(dump, 1, fsize, f); fclose(f); @@ -1332,7 +1348,7 @@ uint64_t hexarray_to_uint64(uint8_t *key) { for (int i = 0;i < 8;i++) sprintf(&temp[(i *2)],"%02X",key[i]); temp[16] = '\0'; - if (sscanf(temp,"%016"llX,&uint_key) < 1) + if (sscanf(temp,"%016"llx,&uint_key) < 1) return 0; return uint_key; } @@ -1431,7 +1447,7 @@ int CmdHFiClassCalcNewKey(const char *Cmd) { errors = param_gethex(tempStr, 0, NEWKEY, dataLen); } else if (dataLen == 1) { keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr <= ICLASS_KEYS_MAX) { + if (keyNbr < ICLASS_KEYS_MAX) { memcpy(NEWKEY, iClass_Key_Table[keyNbr], 8); } else { PrintAndLog("\nERROR: NewKey Nbr is invalid\n"); @@ -1450,7 +1466,7 @@ int CmdHFiClassCalcNewKey(const char *Cmd) { errors = param_gethex(tempStr, 0, OLDKEY, dataLen); } else if (dataLen == 1) { keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr <= ICLASS_KEYS_MAX) { + if (keyNbr < ICLASS_KEYS_MAX) { memcpy(OLDKEY, iClass_Key_Table[keyNbr], 8); } else { PrintAndLog("\nERROR: Credit KeyNbr is invalid\n"); @@ -1498,6 +1514,12 @@ static int loadKeys(char *filename) { long fsize = ftell(f); fseek(f, 0, SEEK_SET); + if ( fsize < 0 ) { + PrintAndLog("Error, when getting filesize"); + fclose(f); + return 1; + } + uint8_t *dump = malloc(fsize); size_t bytes_read = fread(dump, 1, fsize, f); @@ -1590,8 +1612,8 @@ int CmdHFiClassManageKeys(const char *Cmd) { case 'n': case 'N': keyNbr = param_get8(Cmd, cmdp+1); - if (keyNbr < 0) { - PrintAndLog("Wrong block number"); + if (keyNbr >= ICLASS_KEYS_MAX) { + PrintAndLog("Invalid block number"); errors = true; } cmdp += 2; diff --git a/client/cmdhflegic.c b/client/cmdhflegic.c index 7ee601b2..4e52c35c 100644 --- a/client/cmdhflegic.c +++ b/client/cmdhflegic.c @@ -58,7 +58,7 @@ int CmdLegicDecode(const char *Cmd) int crc = 0; int wrp = 0; int wrc = 0; - uint8_t data_buf[1024]; // receiver buffer + uint8_t data_buf[1052]; // receiver buffer char out_string[3076]; // just use big buffer - bad practice char token_type[4]; diff --git a/client/cmdlfem4x.c b/client/cmdlfem4x.c index 7ff8037b..aa0fc856 100644 --- a/client/cmdlfem4x.c +++ b/client/cmdlfem4x.c @@ -21,8 +21,6 @@ #include "cmdlfem4x.h" #include "lfdemod.h" -#define llx PRIx64 - char *global_em410xId; static int CmdHelp(const char *Cmd); @@ -58,7 +56,7 @@ int CmdEM410xRead(const char *Cmd) return 0; } char id[12] = {0x00}; - sprintf(id, "%010llx",lo); + sprintf(id, "%010"PRIx64,lo); global_em410xId = id; return 1; diff --git a/client/cmdlft55xx.c b/client/cmdlft55xx.c index 348cb229..dfee9aa6 100644 --- a/client/cmdlft55xx.c +++ b/client/cmdlft55xx.c @@ -10,7 +10,6 @@ #include #include #include -//#include //not used - marshmellow #include "proxmark3.h" #include "ui.h" #include "graph.h" @@ -22,8 +21,6 @@ #include "util.h" #include "data.h" #include "lfdemod.h" -//#include "../common/crc.h" //not used - marshmellow -//#include "../common/iso14443crc.h" //not used - marshmellow #include "cmdhf14a.h" //for getTagInfo #define T55x7_CONFIGURATION_BLOCK 0x00 @@ -1371,11 +1368,9 @@ int CmdT55xxBruteForce(const char *Cmd) { char buf[9]; char filename[FILE_PATH_SIZE]={0}; int keycnt = 0; + int ch; uint8_t stKeyBlock = 20; - uint8_t *keyBlock = NULL, *p; - keyBlock = calloc(stKeyBlock, 6); - if (keyBlock == NULL) return 1; - + uint8_t *keyBlock = NULL, *p = NULL; uint32_t start_password = 0x00000000; //start password uint32_t end_password = 0xFFFFFFFF; //end password bool found = false; @@ -1383,6 +1378,9 @@ int CmdT55xxBruteForce(const char *Cmd) { char cmdp = param_getchar(Cmd, 0); if (cmdp == 'h' || cmdp == 'H') return usage_t55xx_bruteforce(); + keyBlock = calloc(stKeyBlock, 6); + if (keyBlock == NULL) return 1; + if (cmdp == 'i' || cmdp == 'I') { int len = strlen(Cmd+2); @@ -1417,6 +1415,7 @@ int CmdT55xxBruteForce(const char *Cmd) { if (!p) { PrintAndLog("Cannot allocate memory for defaultKeys"); free(keyBlock); + fclose(f); return 2; } keyBlock = p; @@ -1431,6 +1430,7 @@ int CmdT55xxBruteForce(const char *Cmd) { if (keycnt == 0) { PrintAndLog("No keys found in file"); + free(keyBlock); return 1; } PrintAndLog("Loaded %d keys", keycnt); @@ -1440,8 +1440,10 @@ int CmdT55xxBruteForce(const char *Cmd) { for (uint16_t c = 0; c < keycnt; ++c ) { if (ukbhit()) { - getchar(); + ch = getchar(); + (void)ch; printf("\naborted via keyboard!\n"); + free(keyBlock); return 0; } @@ -1451,6 +1453,7 @@ int CmdT55xxBruteForce(const char *Cmd) { if ( !AquireData(T55x7_PAGE0, T55x7_CONFIGURATION_BLOCK, TRUE, testpwd)) { PrintAndLog("Aquireing data from device failed. Quitting"); + free(keyBlock); return 0; } @@ -1458,10 +1461,12 @@ int CmdT55xxBruteForce(const char *Cmd) { if ( found ) { PrintAndLog("Found valid password: [%08X]", testpwd); + free(keyBlock); return 0; } } PrintAndLog("Password NOT found."); + free(keyBlock); return 0; } @@ -1471,8 +1476,10 @@ int CmdT55xxBruteForce(const char *Cmd) { start_password = param_get32ex(Cmd, 0, 0, 16); end_password = param_get32ex(Cmd, 1, 0, 16); - if ( start_password >= end_password ) return usage_t55xx_bruteforce(); - + if ( start_password >= end_password ) { + free(keyBlock); + return usage_t55xx_bruteforce(); + } PrintAndLog("Search password range [%08X -> %08X]", start_password, end_password); uint32_t i = start_password; @@ -1482,13 +1489,16 @@ int CmdT55xxBruteForce(const char *Cmd) { printf("."); fflush(stdout); if (ukbhit()) { - getchar(); + ch = getchar(); + (void)ch; printf("\naborted via keyboard!\n"); + free(keyBlock); return 0; } if (!AquireData(T55x7_PAGE0, T55x7_CONFIGURATION_BLOCK, TRUE, i)) { PrintAndLog("Aquireing data from device failed. Quitting"); + free(keyBlock); return 0; } found = tryDetectModulation(); @@ -1503,6 +1513,8 @@ int CmdT55xxBruteForce(const char *Cmd) { PrintAndLog("Found valid password: [%08x]", i); else PrintAndLog("Password NOT found. Last tried: [%08x]", --i); + + free(keyBlock); return 0; } diff --git a/client/cmdlfviking.c b/client/cmdlfviking.c index 8c0656d2..45e4b1d5 100644 --- a/client/cmdlfviking.c +++ b/client/cmdlfviking.c @@ -66,7 +66,7 @@ int CmdVikingClone(const char *Cmd) { uint64_t rawID = 0; bool Q5 = false; char cmdp = param_getchar(Cmd, 0); - if (strlen(Cmd) < 0 || cmdp == 'h' || cmdp == 'H') return usage_lf_viking_clone(); + if (strlen(Cmd) == 0 || cmdp == 'h' || cmdp == 'H') return usage_lf_viking_clone(); id = param_get32ex(Cmd, 0, 0, 16); if (id == 0) return usage_lf_viking_clone(); @@ -89,7 +89,7 @@ int CmdVikingSim(const char *Cmd) { uint8_t clk = 32, encoding = 1, separator = 0, invert = 0; char cmdp = param_getchar(Cmd, 0); - if (strlen(Cmd) < 0 || cmdp == 'h' || cmdp == 'H') return usage_lf_viking_sim(); + if (strlen(Cmd) == 0 || cmdp == 'h' || cmdp == 'H') return usage_lf_viking_sim(); id = param_get32ex(Cmd, 0, 0, 16); if (id == 0) return usage_lf_viking_sim(); diff --git a/client/cmdmain.c b/client/cmdmain.c index 7bba80f4..c1d730ee 100644 --- a/client/cmdmain.c +++ b/client/cmdmain.c @@ -177,10 +177,11 @@ void UsbCommandReceived(UsbCommand *UC) switch(UC->cmd) { // First check if we are handling a debug message case CMD_DEBUG_PRINT_STRING: { - char s[USB_CMD_DATA_SIZE+1] = {0x00}; + char s[USB_CMD_DATA_SIZE+1]; + memset(s, 0x00, sizeof(s)); size_t len = MIN(UC->arg[0],USB_CMD_DATA_SIZE); memcpy(s,UC->d.asBytes,len); - PrintAndLog("#db# %s ", s); + PrintAndLog("#db# %s", s); return; } break; diff --git a/client/fpga_compress.c b/client/fpga_compress.c index 2779e835..0c40f22f 100644 --- a/client/fpga_compress.c +++ b/client/fpga_compress.c @@ -91,6 +91,7 @@ int zlib_compress(FILE *infile[], uint8_t num_infiles, FILE *outfile) for(uint16_t j = 0; j < num_infiles; j++) { fclose(infile[j]); } + free(fpga_config); return(EXIT_FAILURE); } @@ -112,7 +113,7 @@ int zlib_compress(FILE *infile[], uint8_t num_infiles, FILE *outfile) compressed_fpga_stream.avail_in = i; compressed_fpga_stream.zalloc = fpga_deflate_malloc; compressed_fpga_stream.zfree = fpga_deflate_free; - + compressed_fpga_stream.opaque = Z_NULL; ret = deflateInit2(&compressed_fpga_stream, COMPRESS_LEVEL, Z_DEFLATED, @@ -187,6 +188,7 @@ int zlib_decompress(FILE *infile, FILE *outfile) compressed_fpga_stream.avail_out = DECOMPRESS_BUF_SIZE; compressed_fpga_stream.zalloc = fpga_deflate_malloc; compressed_fpga_stream.zfree = fpga_deflate_free; + compressed_fpga_stream.opaque = Z_NULL; ret = inflateInit2(&compressed_fpga_stream, 0); @@ -195,9 +197,9 @@ int zlib_decompress(FILE *infile, FILE *outfile) compressed_fpga_stream.next_in = inbuf; uint16_t i = 0; do { - uint8_t c = fgetc(infile); + int c = fgetc(infile); if (!feof(infile)) { - inbuf[i++] = c; + inbuf[i++] = c & 0xFF; compressed_fpga_stream.avail_in++; } else { break; diff --git a/client/loclass/elite_crack.c b/client/loclass/elite_crack.c index c824eaa1..e9814e95 100644 --- a/client/loclass/elite_crack.c +++ b/client/loclass/elite_crack.c @@ -522,8 +522,8 @@ int bruteforceDump(uint8_t dump[], size_t dumpsize, uint16_t keytable[]) errors += bruteforceItem(*attack, keytable); } free(attack); - clock_t t2 = clock(); - float diff = (((float)t2 - (float)t1) / CLOCKS_PER_SEC ); + t1 = clock() - t1; + float diff = ((float)t1 / CLOCKS_PER_SEC ); prnlog("\nPerformed full crack in %f seconds",diff); // Pick out the first 16 bytes of the keytable. @@ -563,15 +563,23 @@ int bruteforceFile(const char *filename, uint16_t keytable[]) long fsize = ftell(f); fseek(f, 0, SEEK_SET); + if (fsize < 0) { + prnlog("Error, when getting fsize"); + fclose(f); + return 1; + } + uint8_t *dump = malloc(fsize); size_t bytes_read = fread(dump, 1, fsize, f); fclose(f); - if (bytes_read < fsize) - { - prnlog("Error, could only read %d bytes (should be %d)",bytes_read, fsize ); - } - return bruteforceDump(dump,fsize,keytable); + if (bytes_read < fsize) { + prnlog("Error, could only read %d bytes (should be %d)",bytes_read, fsize ); + } + + uint8_t res = bruteforceDump(dump,fsize,keytable); + free(dump); + return res; } /** * diff --git a/client/proxmark3.h b/client/proxmark3.h index 8236bfe7..616d9c70 100644 --- a/client/proxmark3.h +++ b/client/proxmark3.h @@ -16,6 +16,7 @@ #include #define llx PRIx64 #define lli PRIi64 +#define llu PRIu64 #define hhu PRIu8 #include "usb_cmd.h" diff --git a/client/util.c b/client/util.c index c4f7d200..e5cbc4aa 100644 --- a/client/util.c +++ b/client/util.c @@ -23,7 +23,7 @@ int ukbhit(void) static struct termios Otty, Ntty; - tcgetattr( 0, &Otty); + if ( tcgetattr( 0, &Otty) == -1 ) return -1; Ntty = Otty; Ntty.c_iflag = 0; /* input mode */ @@ -140,8 +140,9 @@ char *sprint_bin_break(const uint8_t *data, const size_t len, const uint8_t brea size_t in_index = 0; // loop through the out_index to make sure we don't go too far for (size_t out_index=0; out_index < max_len; out_index++) { - // set character - sprintf(tmp++, "%u", data[in_index]); + // set character - (should be binary but verify it isn't more than 1 digit) + if (data[in_index]<10) + sprintf(tmp++, "%u", data[in_index]); // check if a line break is needed and we have room to print it in our array if ( (breaks > 0) && !((in_index+1) % breaks) && (out_index+1 != max_len) ) { // increment and print line break