Merge pull request #162 from marshmellow42/CoverityFixes

Coverity fixes - mainly from @iceman1001 s fork

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;
 }
 
 

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) {
+			if (keyNbr >= ICLASS_KEYS_MAX) {
-				PrintAndLog("Wrong block number");
+				PrintAndLog("Invalid block number");
 				errors = true;
 			}
 			cmdp += 2;

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];
   

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;

client/cmdlft55xx.c

@@ -10,7 +10,6 @@
 #include <stdio.h>
 #include <string.h>
 #include <inttypes.h>
-//#include <time.h>  //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;
+	uint8_t *keyBlock = NULL, *p = NULL;
-	keyBlock = calloc(stKeyBlock, 6);
-	if (keyBlock == NULL) return 1;
-
 	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;
 }
 

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();
 

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;
 

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;

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();
+	t1 = clock() - t1;
-	float diff = (((float)t2 - (float)t1) / CLOCKS_PER_SEC );
+	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)
+	if (bytes_read < fsize) {
-    {
+		prnlog("Error, could only read %d bytes (should be %d)",bytes_read, fsize );
-        prnlog("Error, could only read %d bytes (should be %d)",bytes_read, fsize );
+	}
-    }
+
-	return bruteforceDump(dump,fsize,keytable);
+	uint8_t res = bruteforceDump(dump,fsize,keytable);
+	free(dump);
+	return res;
 }
 /**
  *

client/proxmark3.h

@@ -16,6 +16,7 @@
 #include <inttypes.h>
 #define llx PRIx64
 #define lli PRIi64
+#define llu PRIu64
 #define hhu PRIu8
 
 #include "usb_cmd.h"

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
+		// set character - (should be binary but verify it isn't more than 1 digit)
-		sprintf(tmp++, "%u", data[in_index]);
+		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