miguelbalboa / rfid

Arduino RFID Library for MFRC522
The Unlicense
2.76k stars 1.43k forks source link

Incorrect if statement on UID + BCC byte size #629

Closed lrstolk closed 4 months ago

lrstolk commented 4 months ago

On line 1798/1804 of MFRC522.cpp the if statement and println string tends to be incorrect. It should be 16 as intended.

    // UID + BCC byte can not be larger than 16 together
    if (!newUid || !uidSize || uidSize > 15) {
        if (logErrors) {
            Serial.println(F("New UID buffer empty, size 0, or size > 15 given"));
        }
        return false;
    }

Should be changed to 16

    // UID + BCC byte can not be larger than 16 together
    if (!newUid || !uidSize || uidSize > 16) {
        if (logErrors) {
            Serial.println(F("New UID buffer empty, size 0, or size > 16 given"));
        }
        return false;
    }
Rotzbua commented 4 months ago

The BCC is the checksum and is 1 byte.

UID 15 bytes + BCC 1 byte = 16 bytes

lrstolk commented 4 months ago

The BCC is the checksum and is 1 byte.

UID 15 bytes + BCC 1 byte = 16 bytes

Thank you for you reply! So the last byte of block 0 is the BCC? Cause it did not update when I changed the UID or first 15 bytes.

Rotzbua commented 4 months ago

Yes the last byte should be the BCC. It is just a byte there is a chance that it did not change. If the BCC is wrong the card may not be recognized anymore.

lrstolk commented 4 months ago

Yes the last byte should be the BCC. It is just a byte there is a chance that it did not change. If the BCC is wrong the card may not be recognized anymore.

I see thank you! When I use the MIFARE_SetUid method, I can write 15 bytes right? I didnt find a method to overwrite the manufacture data.

For some context, I am duplicating block 0 for different cards.

I run mfrc522.MIFARE_SetUid(dataBlock, (byte)15, true)

If I calculate the BCC, with the following code, it comes out as 20 . But my card has a 16th byte of 0x02. So something is going wrong.

#include <stdio.h>
#include <stdint.h>

typedef uint8_t byte;

int main() {
    byte block0_buffer[18];

    byte newUid[] = {
        0x8E, 0xC1, 0xB1, 0x7E,
        0x80, 0x08, 0x04, 0x00,
        0x46, 0x59, 0x25, 0x58,
        0x49, 0x10, 0x23
    };

    byte uidSize = (byte)15;

    byte bcc = 0;
    for (uint8_t i = 0; i < uidSize; i++) {
        block0_buffer[i] = newUid[i];
        bcc ^= newUid[i];
    }

    printf("BCC: %u\n", bcc);

    return 0;
}
Rotzbua commented 4 months ago

No need for extra code. There are a lot of tools just search for "Block Check Character Calculator".

it comes out as 20

Avoid decimal representation it can be confusing ;) Result: 0x14

But my card has a 16th byte of 0x02.

The original card? That is strange. Are you sure that it is a classic mifare card? The clone function is old and at that time only classic cards with backdoor were available. Ultra cards are not supported.

Nowadays there a lot of "magic" cards with different features, see https://lab401.com/blogs/academy/know-your-magic-cards

If you just want to quickly clone a card, then I would advise you to use an Android app. As I have never used a clone app, I cannot make a recommendation. For normal reading I recommend the official nxp "taginfo" app.

To be honest, I am no longer actively working on the rfid topic and can therefore only provide limited help.