From 874572d4197e2a02152ea8c2185df90531b576c3 Mon Sep 17 00:00:00 2001 From: "William S. Moses" Date: Sat, 11 Nov 2017 16:08:57 -0500 Subject: [PATCH] Fix memory bounds error --- client/cmdcrc.c | 6 +++--- client/cmdhf.c | 2 +- client/cmdhficlass.c | 36 ++++++++++++++++++------------------ client/cmdhfmf.c | 16 ++++++++-------- client/cmdhfmfu.c | 6 +++--- client/cmdlf.c | 14 +++++++------- client/cmdlfem4x.c | 2 +- client/cmdlfpresco.c | 2 +- client/cmdlft55xx.c | 2 +- client/util.c | 17 ++++++++++++++--- client/util.h | 2 +- 11 files changed, 58 insertions(+), 47 deletions(-) diff --git a/client/cmdcrc.c b/client/cmdcrc.c index 01f65f55..27d081b9 100644 --- a/client/cmdcrc.c +++ b/client/cmdcrc.c @@ -434,9 +434,9 @@ int CmdrevengTestC(const char *Cmd){ char result[30]; int dataLen; char endian = 0; - dataLen = param_getstr(Cmd, cmdp++, inModel); + dataLen = param_getstr(Cmd, cmdp++, inModel, sizeof(inModel)); if (dataLen < 4) return 0; - dataLen = param_getstr(Cmd, cmdp++, inHexStr); + dataLen = param_getstr(Cmd, cmdp++, inHexStr, sizeof(inHexStr)); if (dataLen < 4) return 0; bool reverse = (param_get8(Cmd, cmdp++)) ? true : false; endian = param_getchar(Cmd, cmdp++); @@ -464,7 +464,7 @@ char *SwapEndianStr(const char *inStr, const size_t len, const uint8_t blockSize // takes hex string in and searches for a matching result (hex string must include checksum) int CmdrevengSearch(const char *Cmd){ char inHexStr[50] = {0x00}; - int dataLen = param_getstr(Cmd, 0, inHexStr); + int dataLen = param_getstr(Cmd, 0, inHexStr, sizeof(inHexStr)); if (dataLen < 4) return 0; char *Models[80]; diff --git a/client/cmdhf.c b/client/cmdhf.c index 168296ba..453635b7 100644 --- a/client/cmdhf.c +++ b/client/cmdhf.c @@ -562,7 +562,7 @@ int CmdHFList(const char *Cmd) bool showWaitCycles = false; bool markCRCBytes = false; char type[40] = {0}; - int tlen = param_getstr(Cmd,0,type); + int tlen = param_getstr(Cmd,0,type, sizeof(type)); char param1 = param_getchar(Cmd, 1); char param2 = param_getchar(Cmd, 2); bool errors = false; diff --git a/client/cmdhficlass.c b/client/cmdhficlass.c index 63634cd4..60713a01 100644 --- a/client/cmdhficlass.c +++ b/client/cmdhficlass.c @@ -278,7 +278,7 @@ int CmdHFiClassELoad(const char *Cmd) { //File handling and reading FILE *f; char filename[FILE_PATH_SIZE]; - if(opt == 'f' && param_getstr(Cmd, 1, filename) > 0) + if(opt == 'f' && param_getstr(Cmd, 1, filename, sizeof(filename)) > 0) { f = fopen(filename, "rb"); }else{ @@ -384,7 +384,7 @@ 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, sizeof(filename)) > 0) { f = fopen(filename, "rb"); if ( f == NULL ) { PrintAndLog("Could not find file %s", filename); @@ -605,7 +605,7 @@ int CmdHFiClassReader_Dump(const char *Cmd) { case 'c': case 'C': have_credit_key = true; - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { errors = param_gethex(tempStr, 0, CreditKEY, dataLen); } else if (dataLen == 1) { @@ -629,7 +629,7 @@ int CmdHFiClassReader_Dump(const char *Cmd) { break; case 'f': case 'F': - fileNameLen = param_getstr(Cmd, cmdp+1, filename); + fileNameLen = param_getstr(Cmd, cmdp+1, filename, sizeof(filename)); if (fileNameLen < 1) { PrintAndLog("No filename found after f"); errors = true; @@ -639,7 +639,7 @@ int CmdHFiClassReader_Dump(const char *Cmd) { case 'k': case 'K': have_debit_key = true; - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { @@ -898,7 +898,7 @@ int CmdHFiClass_WriteBlock(const char *Cmd) { break; case 'k': case 'K': - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { @@ -992,7 +992,7 @@ int CmdHFiClassCloneTag(const char *Cmd) { break; case 'f': case 'F': - fileNameLen = param_getstr(Cmd, cmdp+1, filename); + fileNameLen = param_getstr(Cmd, cmdp+1, filename, sizeof(filename)); if (fileNameLen < 1) { PrintAndLog("No filename found after f"); errors = true; @@ -1001,7 +1001,7 @@ int CmdHFiClassCloneTag(const char *Cmd) { break; case 'k': case 'K': - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { @@ -1196,7 +1196,7 @@ int CmdHFiClass_ReadBlock(const char *Cmd) { case 'k': case 'K': auth = true; - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { errors = param_gethex(tempStr, 0, KEY, dataLen); } else if (dataLen == 1) { @@ -1253,7 +1253,7 @@ int CmdHFiClass_loclass(const char *Cmd) { char fileName[255] = {0}; if(opt == 'f') { - if(param_getstr(Cmd, 1, fileName) > 0) + if(param_getstr(Cmd, 1, fileName, sizeof(fileName)) > 0) { return bruteforceFileNoKeys(fileName); }else @@ -1318,14 +1318,14 @@ int CmdHFiClassReadTagFile(const char *Cmd) { char tempnum[5]; FILE *f; char filename[FILE_PATH_SIZE]; - if (param_getstr(Cmd, 0, filename) < 1) + if (param_getstr(Cmd, 0, filename, sizeof(filename)) < 1) return usage_hf_iclass_readtagfile(); - if (param_getstr(Cmd,1,(char *)&tempnum) < 1) + if (param_getstr(Cmd, 1, tempnum, sizeof(tempnum)) < 1) startblock = 0; else sscanf(tempnum,"%d",&startblock); - if (param_getstr(Cmd,2,(char *)&tempnum) < 1) + if (param_getstr(Cmd,2, tempnum, sizeof(tempnum)) < 1) endblock = 0; else sscanf(tempnum,"%d",&endblock); @@ -1458,7 +1458,7 @@ int CmdHFiClassCalcNewKey(const char *Cmd) { return usage_hf_iclass_calc_newkey(); case 'e': case 'E': - dataLen = param_getstr(Cmd, cmdp, tempStr); + dataLen = param_getstr(Cmd, cmdp, tempStr, sizeof(tempStr)); if (dataLen==2) oldElite = true; elite = true; @@ -1466,7 +1466,7 @@ int CmdHFiClassCalcNewKey(const char *Cmd) { break; case 'n': case 'N': - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { errors = param_gethex(tempStr, 0, NEWKEY, dataLen); } else if (dataLen == 1) { @@ -1485,7 +1485,7 @@ int CmdHFiClassCalcNewKey(const char *Cmd) { break; case 'o': case 'O': - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { errors = param_gethex(tempStr, 0, OLDKEY, dataLen); } else if (dataLen == 1) { @@ -1626,7 +1626,7 @@ int CmdHFiClassManageKeys(const char *Cmd) { return usage_hf_iclass_managekeys(); case 'f': case 'F': - fileNameLen = param_getstr(Cmd, cmdp+1, filename); + fileNameLen = param_getstr(Cmd, cmdp+1, filename, sizeof(filename)); if (fileNameLen < 1) { PrintAndLog("No filename found after f"); errors = true; @@ -1645,7 +1645,7 @@ int CmdHFiClassManageKeys(const char *Cmd) { case 'k': case 'K': operation += 3; //set key - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 16) { //ul-c or ev1/ntag key length errors = param_gethex(tempStr, 0, KEY, dataLen); } else { diff --git a/client/cmdhfmf.c b/client/cmdhfmf.c index 553803c1..83060b01 100644 --- a/client/cmdhfmf.c +++ b/client/cmdhfmf.c @@ -533,7 +533,7 @@ static void parseParamTDS(const char *Cmd, const uint8_t indx, bool *paramT, boo char ctmp3[3] = {0}; int len = param_getlength(Cmd, indx); if (len > 0 && len < 4){ - param_getstr(Cmd, indx, ctmp3); + param_getstr(Cmd, indx, ctmp3, sizeof(ctmp3)); *paramT |= (ctmp3[0] == 't' || ctmp3[0] == 'T'); *paramD |= (ctmp3[0] == 'd' || ctmp3[0] == 'D'); @@ -1043,7 +1043,7 @@ int CmdHF14AMfChk(const char *Cmd) // double parameters - ts, ds int clen = param_getlength(Cmd, 2); if (clen == 2 || clen == 3){ - param_getstr(Cmd, 2, ctmp3); + param_getstr(Cmd, 2, ctmp3, sizeof(ctmp3)); ctmp = ctmp3[1]; } //parse @@ -1075,7 +1075,7 @@ int CmdHF14AMfChk(const char *Cmd) keycnt++; } else { // May be a dic file - if ( param_getstr(Cmd, 2 + i,filename) >= FILE_PATH_SIZE ) { + if ( param_getstr(Cmd, 2 + i, filename, sizeof(filename)) >= FILE_PATH_SIZE ) { PrintAndLog("File name too long"); free(keyBlock); return 2; @@ -1398,7 +1398,7 @@ int CmdHF14AMf1kSim(const char *Cmd) { break; case 'f': case 'F': - len = param_getstr(Cmd, cmdp+1, filename); + len = param_getstr(Cmd, cmdp+1, filename, sizeof(filename)); if (len < 1) { PrintAndLog("error no filename found"); return 0; @@ -1674,7 +1674,7 @@ int CmdHF14AMfELoad(const char *Cmd) } } - len = param_getstr(Cmd,nameParamNo,filename); + len = param_getstr(Cmd,nameParamNo,filename,sizeof(filename)); if (len > FILE_PATH_SIZE - 5) len = FILE_PATH_SIZE - 5; @@ -1773,7 +1773,7 @@ int CmdHF14AMfESave(const char *Cmd) } } - len = param_getstr(Cmd,nameParamNo,filename); + len = param_getstr(Cmd,nameParamNo,filename,sizeof(filename)); if (len > FILE_PATH_SIZE - 5) len = FILE_PATH_SIZE - 5; @@ -2137,7 +2137,7 @@ int CmdHF14AMfCLoad(const char *Cmd) } return 0; } else { - param_getstr(Cmd, 0, filename); + param_getstr(Cmd, 0, filename, sizeof(filename)); len = strlen(filename); if (len > FILE_PATH_SIZE - 5) len = FILE_PATH_SIZE - 5; @@ -2348,7 +2348,7 @@ int CmdHF14AMfCSave(const char *Cmd) { } return 0; } else { - param_getstr(Cmd, 0, filename); + param_getstr(Cmd, 0, filename, sizeof(filename)); len = strlen(filename); if (len > FILE_PATH_SIZE - 5) len = FILE_PATH_SIZE - 5; diff --git a/client/cmdhfmfu.c b/client/cmdhfmfu.c index 7dd344e8..63c41728 100644 --- a/client/cmdhfmfu.c +++ b/client/cmdhfmfu.c @@ -703,7 +703,7 @@ int CmdHF14AMfUInfo(const char *Cmd){ return usage_hf_mfu_info(); case 'k': case 'K': - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 32 || dataLen == 8) { //ul-c or ev1/ntag key length errors = param_gethex(tempStr, 0, authenticationkey, dataLen); dataLen /= 2; // handled as bytes from now on @@ -1238,7 +1238,7 @@ int CmdHF14AMfUDump(const char *Cmd){ return usage_hf_mfu_dump(); case 'k': case 'K': - dataLen = param_getstr(Cmd, cmdp+1, tempStr); + dataLen = param_getstr(Cmd, cmdp+1, tempStr, sizeof(tempStr)); if (dataLen == 32 || dataLen == 8) { //ul-c or ev1/ntag key length errors = param_gethex(tempStr, 0, authenticationkey, dataLen); dataLen /= 2; @@ -1256,7 +1256,7 @@ int CmdHF14AMfUDump(const char *Cmd){ break; case 'n': case 'N': - fileNlen = param_getstr(Cmd, cmdp+1, filename); + fileNlen = param_getstr(Cmd, cmdp+1, filename, sizeof(filename)); if (!fileNlen) errors = true; if (fileNlen > FILE_PATH_SIZE-5) fileNlen = FILE_PATH_SIZE-5; cmdp += 2; diff --git a/client/cmdlf.c b/client/cmdlf.c index cdfbee2d..ef9c3cbb 100644 --- a/client/cmdlf.c +++ b/client/cmdlf.c @@ -93,7 +93,7 @@ int CmdLFCommandRead(const char *Cmd) cmdp++; break; case 'c': - param_getstr(Cmd, cmdp+1, (char *)&c.d.asBytes); + param_getstr(Cmd, cmdp+1, (char *)&c.d.asBytes, sizeof(c.d.asBytes)); cmdp+=2; break; case 'd': @@ -491,7 +491,7 @@ int CmdLFfskSim(const char *Cmd) uint8_t fcHigh=0, fcLow=0, clk=0; uint8_t invert=0; bool errors = false; - char hexData[32] = {0x00}; // store entered hex data + char hexData[64] = {0x00}; // store entered hex data uint8_t data[255] = {0x00}; int dataLen = 0; uint8_t cmdp = 0; @@ -522,7 +522,7 @@ int CmdLFfskSim(const char *Cmd) // cmdp++; // break; case 'd': - dataLen = param_getstr(Cmd, cmdp+1, hexData); + dataLen = param_getstr(Cmd, cmdp+1, hexData, sizeof(hexData)); if (dataLen==0) { errors=true; } else { @@ -593,7 +593,7 @@ int CmdLFaskSim(const char *Cmd) uint8_t encoding = 1, separator = 0; uint8_t clk=0, invert=0; bool errors = false; - char hexData[32] = {0x00}; + char hexData[64] = {0x00}; uint8_t data[255]= {0x00}; // store entered hex data int dataLen = 0; uint8_t cmdp = 0; @@ -628,7 +628,7 @@ int CmdLFaskSim(const char *Cmd) cmdp++; break; case 'd': - dataLen = param_getstr(Cmd, cmdp+1, hexData); + dataLen = param_getstr(Cmd, cmdp+1, hexData, sizeof(hexData)); if (dataLen==0) { errors=true; } else { @@ -687,7 +687,7 @@ int CmdLFpskSim(const char *Cmd) uint8_t carrier=0, clk=0; uint8_t invert=0; bool errors = false; - char hexData[32] = {0x00}; // store entered hex data + char hexData[64] = {0x00}; // store entered hex data uint8_t data[255] = {0x00}; int dataLen = 0; uint8_t cmdp = 0; @@ -723,7 +723,7 @@ int CmdLFpskSim(const char *Cmd) cmdp++; break; case 'd': - dataLen = param_getstr(Cmd, cmdp+1, hexData); + dataLen = param_getstr(Cmd, cmdp+1, hexData, sizeof(hexData)); if (dataLen==0) { errors=true; } else { diff --git a/client/cmdlfem4x.c b/client/cmdlfem4x.c index e6a25764..6d562be0 100644 --- a/client/cmdlfem4x.c +++ b/client/cmdlfem4x.c @@ -335,7 +335,7 @@ int CmdEM410xBrute(const char *Cmd) delay = param_get32ex(Cmd, 4, 1000, 10); } - param_getstr(Cmd, 0, filename); + param_getstr(Cmd, 0, filename, sizeof(filename)); uidBlock = calloc(stUidBlock, 5); if (uidBlock == NULL) return 1; diff --git a/client/cmdlfpresco.c b/client/cmdlfpresco.c index 29fc81f2..2f4bacfe 100644 --- a/client/cmdlfpresco.c +++ b/client/cmdlfpresco.c @@ -71,7 +71,7 @@ int GetWiegandFromPresco(const char *Cmd, uint32_t *sitecode, uint32_t *usercode case 'D': case 'd': //param get string int param_getstr(const char *line, int paramnum, char * str) - stringlen = param_getstr(Cmd, cmdp+1, id); + stringlen = param_getstr(Cmd, cmdp+1, id, sizeof(id)); if (stringlen < 2) return -1; cmdp+=2; break; diff --git a/client/cmdlft55xx.c b/client/cmdlft55xx.c index 252aaaa2..92a00bce 100644 --- a/client/cmdlft55xx.c +++ b/client/cmdlft55xx.c @@ -236,7 +236,7 @@ int CmdT55xxSetConfig(const char *Cmd) { cmdp+=2; break; case 'd': - param_getstr(Cmd, cmdp+1, modulation); + param_getstr(Cmd, cmdp+1, modulation, sizeof(modulation)); cmdp += 2; if ( strcmp(modulation, "FSK" ) == 0) { diff --git a/client/util.c b/client/util.c index 9dab4655..de62ac79 100644 --- a/client/util.c +++ b/client/util.c @@ -531,11 +531,19 @@ int param_gethex_to_eol(const char *line, int paramnum, uint8_t * data, int maxd return 0; } -int param_getstr(const char *line, int paramnum, char * str) +int param_getstr(const char *line, int paramnum, char * str, size_t buffersize) { int bg, en; - if (param_getptr(line, &bg, &en, paramnum)) return 0; + if (param_getptr(line, &bg, &en, paramnum)) { + return 0; + } + + // Prevent out of bounds errors + if (en - bg + 1 >= buffersize) { + printf("out of bounds error: want %lu bytes have %lu bytes\n", en - bg + 1 + 1, buffersize); + return 0; + } memcpy(str, line + bg, en - bg + 1); str[en - bg + 1] = 0; @@ -553,6 +561,7 @@ https://github.com/ApertureLabsLtd/RFIDler/blob/master/firmware/Pic32/RFIDler.X/ int hextobinarray(char *target, char *source) { int length, i, count= 0; + char* start = source; char x; length = strlen(source); @@ -568,8 +577,10 @@ int hextobinarray(char *target, char *source) x -= '0'; else if (x >= 'A' && x <= 'F') x -= 'A' - 10; - else + else { + printf("Discovered unknown character %c %d at idx %d of %s\n", x, x, source - start, start); return 0; + } // output for(i= 0 ; i < 4 ; ++i, ++count) *(target++)= (x >> (3 - i)) & 1; diff --git a/client/util.h b/client/util.h index f42625b1..8bab8cfd 100644 --- a/client/util.h +++ b/client/util.h @@ -64,7 +64,7 @@ extern uint8_t param_isdec(const char *line, int paramnum); extern int param_gethex(const char *line, int paramnum, uint8_t * data, int hexcnt); extern int param_gethex_ex(const char *line, int paramnum, uint8_t * data, int *hexcnt); extern int param_gethex_to_eol(const char *line, int paramnum, uint8_t * data, int maxdatalen, int *datalen); -extern int param_getstr(const char *line, int paramnum, char * str); +extern int param_getstr(const char *line, int paramnum, char * str, size_t buffersize); extern int hextobinarray( char *target, char *source); extern int hextobinstring( char *target, char *source); -- 2.39.5