skjolber / desfire-tools-for-android

Open source MIFARE DESFire EV1 NFC library for Android
80 stars 22 forks source link

Length error in create_file2 method #4

Closed AndroidCrypto closed 1 year ago

AndroidCrypto commented 1 year ago

First let me say "thank you" for this fine peace of work, at the moment I'm trying to enhance your sample app by adding some functions to it (e.g. create applications and files, deletion, formatting and writing to the different file types).

I created a standard file with no problems but when trying to create a cyclic record file I got an "7E" (= length) error. When analyzing the code I found that a record file is created using this method (located in com.github.skjolber.desfire.libfreefare.MifareDesfire.java):

public static int
create_file2 (MifareTag tag, byte command, byte file_no, boolean has_iso_file_id, short iso_file_id, byte communication_settings, short access_rights, int record_size, int max_number_of_records) throws Exception
{
    // todo the original create_file2 method seems to use wrong lengths
    ASSERT_ACTIVE (tag);
    ASSERT_MIFARE_DESFIRE (tag);

    ByteBuffer cmd = C.BUFFER_INIT (11 + CMAC_LENGTH);

    C.BUFFER_APPEND (cmd, command);
    C.BUFFER_APPEND (cmd, file_no);
    if (has_iso_file_id)

        C.BUFFER_APPEND_LE (cmd, C.getBytes2(iso_file_id), 2, 2);
    C.BUFFER_APPEND (cmd, communication_settings);
    C.BUFFER_APPEND_LE (cmd, C.getBytes2(access_rights), 2, 2);
    C.BUFFER_APPEND_LE (cmd, C.getBytes4(record_size), 3, 4); // this might be wrong, field size is 3 ?
    C.BUFFER_APPEND_LE (cmd, C.getBytes4(max_number_of_records), 3, 4); // this might be wrong, field size is 3 ?

    byte[] p = MifareDesfireCrypto.mifare_cryto_preprocess_data (tag, cmd, cmd.position(), 0, MifareDesfireCrypto.MDCM_PLAIN | MifareDesfireCrypto.CMAC_COMMAND);

    byte[] res = DESFIRE_TRANSCEIVE2(tag, p);

    byte[] buffer = new byte[1 + CMAC_LENGTH];
    System.arraycopy(res, 0, buffer, 0, res.length);

    p = MifareDesfireCrypto.mifare_cryto_postprocess_data (tag, buffer, res.length, MifareDesfireCrypto.MDCM_PLAIN | MifareDesfireCrypto.CMAC_COMMAND | MifareDesfireCrypto.CMAC_VERIFY);

    if (p == null) {
        return -1;
    }

    cached_file_settings[file_no] = null;

    return 0;
}

As you can see in my code comments, the C.BUFFER_APPEND_LE (cmd, C.getBytes4(record_size), 3, 4) and C.BUFFER_APPEND_LE (cmd, C.getBytes4(max_number_of_records), 3, 4) are using a fileSize of 4 instead of 3. In my opinion this is a "copy&paste" issue when working with previous create_value_file method.

I changed the method in my code but it could be a good idea to change your code as well, thanks again.

For all others: in the meantime you change the mentioned code lines as follows (change to C.getBytes3 as well):

    C.BUFFER_APPEND_LE (cmd, C.getBytes3(record_size), 3, 3); // changed to field size is 3
    C.BUFFER_APPEND_LE (cmd, C.getBytes3(max_number_of_records), 3, 3); // changed to field size is 3

Greetings from Germany Michael

skjolber commented 1 year ago

Thanks @AndroidCrypto, I've updated the source code with your fixes. :+1: