nfc-tools / libfreefare

A convenience API for NFC cards manipulations on top of libnfc.
Other
407 stars 105 forks source link

Error when free() buffer in read_data() function #99

Open crossmax opened 5 years ago

crossmax commented 5 years ago

Hi, In my app I used nfc_initiator_transceive_bytes() (libnfc) function to read Desfire tag and building each APDU but in my tests, the reading that uses the read_data() (libfreefare) function is faster. However, it gives me many errors when read_data() free the read data buffer:

*** Error in `./app': free(): invalid next size (normal): 0x00c65588 ***

The implicated fragment is:

uint8_t *read_buffer = malloc(enciphered_data_length(tag, length * record_size, 0) + 1);

do {
DESFIRE_TRANSCEIVE2(tag, p, __cmd_n, res);

size_t frame_bytes = BUFFER_SIZE(res) - 1;
memcpy(read_buffer + bytes_received, res, frame_bytes);
bytes_received += frame_bytes;

p[0] = 0xAF;
__cmd_n = 1;
} while (0xAF == res[__res_n - 1]);

read_buffer[bytes_received++] = 0x00;

ssize_t sr = bytes_received;
p = mifare_cryto_postprocess_data(tag, read_buffer, &sr, cs | CMAC_COMMAND | CMAC_VERIFY | MAC_VERIFY);

if (sr > 0)
memcpy(data, read_buffer, sr - 1);

free(read_buffer);

I read that the problem was solved if change a line in mifare_desfire_crypto.c file, in enciphered_data_length() function exactly: return padded_data_length (nbytes + crc_length + 1, block_size); instead return padded_data_length (nbytes + crc_length, block_size); But the error persist.

Anyone know what can I do with this issue?

darconeous commented 5 years ago

Looks like there is some sort of memory corruption going on. Possibly related to #114?

alenloncaric commented 2 years ago

This issue creates segmentation faults, the read buffer is not long enough by 2-4 bytes... causes memory corruption.

`
int read_buffer_len = enciphered_data_length(tag, length record_size, 0) + 1 + 4 ; uint8_t read_buffer = malloc(read_buffer_len); memset(read_buffer, 0, read_buffer_len);

do {

    if ((rc = MIFARE_DESFIRE_TRANSCEIVE(tag, p, __cmd_n, res, __res_size, &__res_n)) < 0) {
        free(read_buffer);
        return rc;
    }

    size_t frame_bytes = BUFFER_SIZE(res) - 1;

    if (frame_bytes + bytes_received > read_buffer_len)
    {
        read_buffer_len = frame_bytes + bytes_received;

        read_buffer = realloc(read_buffer,read_buffer_len);
        if(read_buffer == NULL)
            return errno = EINVAL, -1;
    }

    memcpy(read_buffer + bytes_received, res, frame_bytes);
    bytes_received += frame_bytes;

    p[0] = 0xAF;
    __cmd_n = 1;
} while (0xAF == res[__res_n - 1]);

read_buffer[bytes_received++] = 0x00;

ssize_t sr = bytes_received;
p = mifare_cryto_postprocess_data(tag, read_buffer, &sr, cs | CMAC_COMMAND | CMAC_VERIFY | MAC_VERIFY);`

added +4 bytes which solved my issues, while adding realloc if ever would come over buffersize