From 05fd71264a25efd6d29d82d94bb7ddc447251fc0 Mon Sep 17 00:00:00 2001 From: iceman1001 Date: Wed, 20 Jan 2016 13:03:49 +0100 Subject: [PATCH 1/2] FIX: Coverity Scan complains that the realloc might fail.. --- client/reveng/reveng.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/reveng/reveng.c b/client/reveng/reveng.c index 5cda62d16..d82353f8a 100644 --- a/client/reveng/reveng.c +++ b/client/reveng/reveng.c @@ -473,9 +473,12 @@ chkres(int *resc, model_t **result, const poly_t divisor, const poly_t init, int pfree(&xor); if(aptr != eptr) return; - if(!(*result = realloc(*result, ++*resc * sizeof(model_t)))) + *result = realloc(*result, ++*resc * sizeof(model_t)); + if (!*result) { uerror("cannot reallocate result array"); - + return; + } + rptr = *result + *resc - 1; rptr->spoly = pclone(divisor); rptr->init = pclone(init); From 4a71da5a359c5f2449a19f19f2fbcbe7bb989ebb Mon Sep 17 00:00:00 2001 From: iceman1001 Date: Wed, 20 Jan 2016 17:13:25 +0100 Subject: [PATCH 2/2] FIX: Coverity Scan complains about resourceleaks, array reads outside of index, uninitialized variables. --- armsrc/desfire_crypto.c | 1 + armsrc/fpgaloader.c | 35 ++++------ armsrc/iclass.c | 150 ++++++++++++++++++---------------------- armsrc/iso14443a.c | 12 ++-- armsrc/iso15693.c | 5 +- armsrc/mifarecmd.c | 4 +- 6 files changed, 94 insertions(+), 113 deletions(-) diff --git a/armsrc/desfire_crypto.c b/armsrc/desfire_crypto.c index 8bb803483..469a8ec46 100644 --- a/armsrc/desfire_crypto.c +++ b/armsrc/desfire_crypto.c @@ -84,6 +84,7 @@ void cmac (const desfirekey_t key, uint8_t *ivect, const uint8_t *data, size_t l mifare_cypher_blocks_chained (NULL, key, ivect, buffer, len, MCD_SEND, MCO_ENCYPHER); memcpy (cmac, ivect, kbs); + free(buffer); } size_t key_block_size (const desfirekey_t key) { diff --git a/armsrc/fpgaloader.c b/armsrc/fpgaloader.c index 7bcee1eb6..da85c66c1 100644 --- a/armsrc/fpgaloader.c +++ b/armsrc/fpgaloader.c @@ -158,9 +158,8 @@ void FpgaSetupSsc(void) //----------------------------------------------------------------------------- bool FpgaSetupSscDma(uint8_t *buf, int len) { - if (buf == NULL) { + if (buf == NULL) return false; - } AT91C_BASE_PDC_SSC->PDC_PTCR = AT91C_PDC_RXTDIS; // Disable DMA Transfer AT91C_BASE_PDC_SSC->PDC_RPR = (uint32_t) buf; // transfer to this memory address @@ -184,15 +183,15 @@ static int get_from_fpga_combined_stream(z_streamp compressed_fpga_stream, uint8 compressed_fpga_stream->avail_out = OUTPUT_BUFFER_LEN; fpga_image_ptr = output_buffer; int res = inflate(compressed_fpga_stream, Z_SYNC_FLUSH); - if (res != Z_OK) { + + if (res != Z_OK) Dbprintf("inflate returned: %d, %s", res, compressed_fpga_stream->msg); - } - if (res < 0) { + + if (res < 0) return res; - } } - uncompressed_bytes_cnt++; + ++uncompressed_bytes_cnt; return *fpga_image_ptr++; } @@ -209,8 +208,7 @@ static int get_from_fpga_stream(int bitstream_version, z_streamp compressed_fpga get_from_fpga_combined_stream(compressed_fpga_stream, output_buffer); } - return get_from_fpga_combined_stream(compressed_fpga_stream, output_buffer); - + return get_from_fpga_combined_stream(compressed_fpga_stream, output_buffer); } @@ -247,16 +245,14 @@ static bool reset_fpga_stream(int bitstream_version, z_streamp compressed_fpga_s fpga_image_ptr = output_buffer; - for (uint16_t i = 0; i < FPGA_BITSTREAM_FIXED_HEADER_SIZE; i++) { + for (uint16_t i = 0; i < FPGA_BITSTREAM_FIXED_HEADER_SIZE; i++) header[i] = get_from_fpga_stream(bitstream_version, compressed_fpga_stream, output_buffer); - } // Check for a valid .bit file (starts with _bitparse_fixed_header) - if(memcmp(_bitparse_fixed_header, header, FPGA_BITSTREAM_FIXED_HEADER_SIZE) == 0) { + if(memcmp(_bitparse_fixed_header, header, FPGA_BITSTREAM_FIXED_HEADER_SIZE) == 0) return true; - } else { - return false; - } + + return false; } @@ -413,7 +409,7 @@ static int bitparse_find_section(int bitstream_version, char section_name, unsig void FpgaDownloadAndGo(int bitstream_version) { z_stream compressed_fpga_stream; - uint8_t output_buffer[OUTPUT_BUFFER_LEN]; + uint8_t output_buffer[OUTPUT_BUFFER_LEN] = {0x00}; // check whether or not the bitstream is already loaded if (downloaded_bitstream == bitstream_version) @@ -447,18 +443,17 @@ void FpgaDownloadAndGo(int bitstream_version) void FpgaGatherVersion(int bitstream_version, char *dst, int len) { unsigned int fpga_info_len; - char tempstr[40]; + char tempstr[40] = {0x00}; z_stream compressed_fpga_stream; - uint8_t output_buffer[OUTPUT_BUFFER_LEN]; + uint8_t output_buffer[OUTPUT_BUFFER_LEN] = {0x00}; dst[0] = '\0'; // ensure that we can allocate enough memory for decompression: BigBuf_free(); - if (!reset_fpga_stream(bitstream_version, &compressed_fpga_stream, output_buffer)) { + if (!reset_fpga_stream(bitstream_version, &compressed_fpga_stream, output_buffer)) return; - } if(bitparse_find_section(bitstream_version, 'a', &fpga_info_len, &compressed_fpga_stream, output_buffer)) { for (uint16_t i = 0; i < fpga_info_len; i++) { diff --git a/armsrc/iclass.c b/armsrc/iclass.c index 2dff8a4e7..7a68ea6bc 100644 --- a/armsrc/iclass.c +++ b/armsrc/iclass.c @@ -633,8 +633,6 @@ static RAMFUNC int ManchesterDecoding(int v) //----------------------------------------------------------------------------- void RAMFUNC SnoopIClass(void) { - - // We won't start recording the frames that we acquire until we trigger; // a good trigger condition to get started is probably when we see a // response from the tag. @@ -705,22 +703,22 @@ void RAMFUNC SnoopIClass(void) for(;;) { LED_A_ON(); WDT_HIT(); - int behindBy = (lastRxCounter - AT91C_BASE_PDC_SSC->PDC_RCR) & - (DMA_BUFFER_SIZE-1); - if(behindBy > maxBehindBy) { + int behindBy = (lastRxCounter - AT91C_BASE_PDC_SSC->PDC_RCR) & (DMA_BUFFER_SIZE-1); + + if ( behindBy > maxBehindBy) { maxBehindBy = behindBy; - if(behindBy > (9 * DMA_BUFFER_SIZE / 10)) { + if ( behindBy > (9 * DMA_BUFFER_SIZE / 10)) { Dbprintf("blew circular buffer! behindBy=0x%x", behindBy); goto done; } } - if(behindBy < 1) continue; + if( behindBy < 1) continue; - LED_A_OFF(); + LED_A_OFF(); smpl = upTo[0]; upTo++; lastRxCounter -= 1; - if(upTo - dmaBuf > DMA_BUFFER_SIZE) { + if (upTo - dmaBuf > DMA_BUFFER_SIZE) { upTo -= DMA_BUFFER_SIZE; lastRxCounter += DMA_BUFFER_SIZE; AT91C_BASE_PDC_SSC->PDC_RNPR = (uint32_t) upTo; @@ -728,77 +726,75 @@ void RAMFUNC SnoopIClass(void) } //samples += 4; - samples += 1; + samples += 1; - if(smpl & 0xF) { - decbyte ^= (1 << (3 - div)); - } + if(smpl & 0xF) + decbyte ^= (1 << (3 - div)); - // FOR READER SIDE COMMUMICATION... - - decbyter <<= 2; - decbyter ^= (smpl & 0x30); - - div++; - if((div + 1) % 2 == 0) { - smpl = decbyter; - if(OutOfNDecoding((smpl & 0xF0) >> 4)) { - rsamples = samples - Uart.samples; - time_stop = (GetCountSspClk()-time_0) << 4; - LED_C_ON(); + // FOR READER SIDE COMMUMICATION... - //if(!LogTrace(Uart.output,Uart.byteCnt, rsamples, Uart.parityBits,TRUE)) break; - //if(!LogTrace(NULL, 0, Uart.endTime*16 - DELAY_READER_AIR2ARM_AS_SNIFFER, 0, TRUE)) break; - if(tracing) { - uint8_t parity[MAX_PARITY_SIZE]; - GetParity(Uart.output, Uart.byteCnt, parity); - LogTrace(Uart.output,Uart.byteCnt, time_start, time_stop, parity, TRUE); + decbyter <<= 2; + decbyter ^= (smpl & 0x30); + + ++div; + + if (( div + 1) % 2 == 0) { + smpl = decbyter; + if ( OutOfNDecoding((smpl & 0xF0) >> 4)) { + rsamples = samples - Uart.samples; + time_stop = (GetCountSspClk()-time_0) << 4; + LED_C_ON(); + + //if(!LogTrace(Uart.output,Uart.byteCnt, rsamples, Uart.parityBits,TRUE)) break; + //if(!LogTrace(NULL, 0, Uart.endTime*16 - DELAY_READER_AIR2ARM_AS_SNIFFER, 0, TRUE)) break; + if(tracing) { + uint8_t parity[MAX_PARITY_SIZE]; + GetParity(Uart.output, Uart.byteCnt, parity); + LogTrace(Uart.output,Uart.byteCnt, time_start, time_stop, parity, TRUE); + } + + /* And ready to receive another command. */ + Uart.state = STATE_UNSYNCD; + /* And also reset the demod code, which might have been */ + /* false-triggered by the commands from the reader. */ + Demod.state = DEMOD_UNSYNCD; + LED_B_OFF(); + Uart.byteCnt = 0; + } else { + time_start = (GetCountSspClk()-time_0) << 4; } - - - /* And ready to receive another command. */ - Uart.state = STATE_UNSYNCD; - /* And also reset the demod code, which might have been */ - /* false-triggered by the commands from the reader. */ - Demod.state = DEMOD_UNSYNCD; - LED_B_OFF(); - Uart.byteCnt = 0; - }else{ - time_start = (GetCountSspClk()-time_0) << 4; + decbyter = 0; } - decbyter = 0; - } - if(div > 3) { - smpl = decbyte; - if(ManchesterDecoding(smpl & 0x0F)) { - time_stop = (GetCountSspClk()-time_0) << 4; + if(div > 3) { + smpl = decbyte; + if(ManchesterDecoding(smpl & 0x0F)) { + time_stop = (GetCountSspClk()-time_0) << 4; - rsamples = samples - Demod.samples; - LED_B_ON(); + rsamples = samples - Demod.samples; + LED_B_ON(); - if(tracing) { - uint8_t parity[MAX_PARITY_SIZE]; - GetParity(Demod.output, Demod.len, parity); - LogTrace(Demod.output, Demod.len, time_start, time_stop, parity, FALSE); + if(tracing) { + uint8_t parity[MAX_PARITY_SIZE]; + GetParity(Demod.output, Demod.len, parity); + LogTrace(Demod.output, Demod.len, time_start, time_stop, parity, FALSE); + } + + // And ready to receive another response. + memset(&Demod, 0, sizeof(Demod)); + Demod.output = tagToReaderResponse; + Demod.state = DEMOD_UNSYNCD; + LED_C_OFF(); + } else { + time_start = (GetCountSspClk()-time_0) << 4; } - - // And ready to receive another response. - memset(&Demod, 0, sizeof(Demod)); - Demod.output = tagToReaderResponse; - Demod.state = DEMOD_UNSYNCD; - LED_C_OFF(); - }else{ - time_start = (GetCountSspClk()-time_0) << 4; + + div = 0; + decbyte = 0x00; } - - div = 0; - decbyte = 0x00; - } - //} - if(BUTTON_PRESS()) { + if (BUTTON_PRESS()) { DbpString("cancelled_a"); goto done; } @@ -813,18 +809,14 @@ done: AT91C_BASE_PDC_SSC->PDC_PTCR = AT91C_PDC_RXTDIS; Dbprintf("%x %x %x", maxBehindBy, Uart.state, Uart.byteCnt); Dbprintf("%x %x %x", Uart.byteCntMax, BigBuf_get_traceLen(), (int)Uart.output[0]); - LED_A_OFF(); - LED_B_OFF(); - LED_C_OFF(); - LED_D_OFF(); + LEDsoff(); set_tracing(FALSE); } void rotateCSN(uint8_t* originalCSN, uint8_t* rotatedCSN) { int i; - for(i = 0; i < 8; i++) { + for(i = 0; i < 8; i++) rotatedCSN[i] = (originalCSN[i] >> 3) | (originalCSN[(i+1)%8] << 5); - } } //----------------------------------------------------------------------------- @@ -1339,15 +1331,11 @@ int doIClassSimulation( int simulationMode, uint8_t *reader_mac_buf) } } - //Dbprintf("%x", cmdsRecvd); - LED_A_OFF(); - LED_B_OFF(); - LED_C_OFF(); - + LEDsoff(); + if(buttonPressed) - { DbpString("Button pressed"); - } + return buttonPressed; } diff --git a/armsrc/iso14443a.c b/armsrc/iso14443a.c index 8e1b7b7d6..bfd7069b4 100644 --- a/armsrc/iso14443a.c +++ b/armsrc/iso14443a.c @@ -2399,10 +2399,10 @@ void ReaderMifare(bool first_try) isOK = -4; // Card's PRNG runs at an unexpected frequency or resets unexpectedly break; } else { // continue for a while, just to collect some debug info - debug_info[strategy][debug_info_nr] = nt_distance; - debug_info_nr++; + ++debug_info_nr; + debug_info[strategy][debug_info_nr] = nt_distance; if (debug_info_nr == NUM_DEBUG_INFOS) { - strategy++; + ++strategy; debug_info_nr = 0; } continue; @@ -2427,7 +2427,7 @@ void ReaderMifare(bool first_try) } catch_up_cycles /= elapsed_prng_sequences; if (catch_up_cycles == last_catch_up) { - consecutive_resyncs++; + ++consecutive_resyncs; } else { last_catch_up = catch_up_cycles; @@ -2488,8 +2488,8 @@ void ReaderMifare(bool first_try) if (isOK == -4) { if (MF_DBGLEVEL >= 3) { - for (uint16_t i = 0; i <= MAX_STRATEGY; i++) { - for(uint16_t j = 0; j < NUM_DEBUG_INFOS; j++) { + for (uint16_t i = 0; i <= MAX_STRATEGY; ++i) { + for(uint16_t j = 0; j < NUM_DEBUG_INFOS; ++j) { Dbprintf("collected debug info[%d][%d] = %d", i, j, debug_info[i][j]); } } diff --git a/armsrc/iso15693.c b/armsrc/iso15693.c index 928439aa1..175015267 100644 --- a/armsrc/iso15693.c +++ b/armsrc/iso15693.c @@ -903,10 +903,7 @@ int SendDataTag(uint8_t *send, int sendlen, int init, int speed, uint8_t **recv) *recv=answer; } - LED_A_OFF(); - LED_B_OFF(); - LED_C_OFF(); - LED_D_OFF(); + LEDsoff(); return answerLen; } diff --git a/armsrc/mifarecmd.c b/armsrc/mifarecmd.c index 91aa72185..81bbb355b 100644 --- a/armsrc/mifarecmd.c +++ b/armsrc/mifarecmd.c @@ -608,7 +608,7 @@ void MifareAcquireEncryptedNonces(uint32_t arg0, uint32_t arg1, uint32_t flags, { uint64_t ui64Key = 0; uint8_t uid[10]; - uint32_t cuid; + uint32_t cuid = 0; uint8_t cascade_levels = 0; struct Crypto1State mpcs = {0, 0}; struct Crypto1State *pcs; @@ -1348,7 +1348,7 @@ void MifareCollectNonces(uint32_t arg0, uint32_t arg1){ void Mifare_DES_Auth1(uint8_t arg0, uint8_t *datain){ - byte_t dataout[11] = {0x00}; + byte_t dataout[12] = {0x00}; uint8_t uid[10] = {0x00}; uint32_t cuid = 0x00;