mifare_desfire.c: buffer overflow at read_data

If size_t length ist >52 and is dividable by 4 but not by 8 (like 
60,68,...,876,...)
enciphered_data_length results in wrong padded data lenght and the read_buffer 
is
to small to keep the content.

Here is a q&d printf debug output for your convenience with 60 as length

+++++++++++++++ ENTER read_data command 189 file_no 7 offset 0 lenght 60 cs 1
############### ENTER padded_data_length: nbytes 8 block_size 16
nbytes % block_size)
############### IF branch : (nbytes / block_size) 0
############### IF branch : ((nbytes / block_size) + 1) 1
############### ((nbytes / block_size) + 1) * block_size 16
=============== ENTER enciphered_data_length
=============== nbytes 60, crc_length 4, block_size 16
############### ENTER padded_data_length: nbytes 64 block_size 16
nbytes % block_size)
############### ELSE: nbytes 64
=============== padded_data_length 64
=============== EXIT enciphered_data_length
+++++++++++++++ enciphered_data_length (tag, length, 0) = 64 rember the + 1 
later
+++++++++++++++ bytes_received before memcpy = 0
+++++++++++++++ bytes_received after memcpy = 48
+++++++++++++++ bytes_received before memcpy = 48
+++++++++++++++ bytes_received after memcpy = 68
+++++++++++++++ bytes_received after transfer 68 remvember the ++ later on
+++++++++++++++ sr = 69
############### ENTER padded_data_length: nbytes 61 block_size 16
nbytes % block_size)
 ############### IF branch : (nbytes / block_size) 3
############### IF branch : ((nbytes / block_size) + 1) 4
############### ((nbytes / block_size) + 1) * block_size 64
*** Error in `/test': malloc(): memory corruption: 0x00e2e238 ***

Increasing the padding by one at enciphered_data_length (padded_data_length 
(nbytes + crc_length +1, block_size)
instead of (padded_data_length (nbytes + crc_length, block_size)) solves the 
problem for me. And I attached a patch
for it. But I am not sure if this patch will be right in every case. In the 
case above the resulting buffer size is
80 which is enough to store the data.

While reading the source I also came along that malloc is not checked for 
errors.

Also this part makes me wonder if you wanted to use
    uint8_t ocs = cs;
    if ((MIFARE_DESFIRE (tag)->session_key) && (cs | MDCM_MACED)) {
    switch (MIFARE_DESFIRE (tag)->authentication_scheme) {
    case AS_LEGACY:
        break;
    case AS_NEW:
        cs = MDCM_PLAIN;
        break;
    }
    }
    uint8_t *p = mifare_cryto_preprocess_data (tag, cmd, &__cmd_n, 8, MDCM_PLAIN | CMAC_COMMAND);
    cs = ocs;

mifare_cryto_preprocess_data (tag, cmd, &__cmd_n, 8, cs | CMAC_COMMAND)
instead of
mifare_cryto_preprocess_data (tag, cmd, &__cmd_n, 8, MDCM_PLAIN | CMAC_COMMAND)

Thanks in advance,
Bernhard
Original issue reported on code.google.com by hale.dev...@gmail.com on 4 Nov 2014 at 3:41
Attachments:
0001-mifare_desfire_crypto.c-fix-wrong-data-length.patch
timmerk / libfreefare

mifare_desfire.c: buffer overflow at read_data #33
