Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix facility code and card number checking in LF HID Brute #2741

Merged
merged 2 commits into from
Feb 3, 2025

Conversation

Donny-Guo
Copy link
Contributor

In lf hid brute command, there's a checking on the range of facility code and card number to determine when it should stop. The original code (card_hi.FacilityCode < 0xFF and card_hi.CardNumber < 0xFFFF) does not account for different card formats.

Fix it by pulling the corresponding card format limit from FormatTable.

Copy link

github-actions bot commented Feb 3, 2025

You are welcome to add an entry to the CHANGELOG.md as well

@iceman1001
Copy link
Collaborator

  1. Don't remove a section code without any comment. I don't think you meant to.
  2. This solution feels wrong. Where I agree that the format in use should need a better struct, I don't see adding the extra struct meaningful. It can easily be added to the first struct. I would use names like "MaxFC", "MaxCN", etc.
  3. This should then be modified in all the unpack and pack functions. And other places where code like this is being used.

@Donny-Guo
Copy link
Contributor Author

  1. I am really sorry for my silly mistake and the missing code is recovered.
  2. I've removed the new struct and add variables to the first struct.
  3. There's no changes to all pack and unpack functions for now since both FormatTable and pack/unpack functions are all static and I could not find a way to do such modification without making big changes. I am considering changing all pack/unpack functions to non-static but I am not sure how this will affect the proxmark image size or anything else. Any suggestions?

@iceman1001
Copy link
Collaborator

Now its a better solution.

When I mentioned pack/unpack functions it has nothing to do with being static or not.
It has to do with the checks that are inside each function. They should get its designated format from the table of formats and then implement range checks accordingly. You follow me?

static bool Pack_Tecom27(wiegand_card_t *card, wiegand_message_t *packed, bool preamble) {
    memset(packed, 0, sizeof(wiegand_message_t));

    if (card->FacilityCode > 0x7FF) return false; // Can't encode FC.
    if (card->CardNumber > 0xFFFF) return false; // Can't encode CN.
    if (card->IssueLevel > 0) return false; // Not used in this format
    if (card->OEM > 0) return false; // Not used in this format

@iceman1001 iceman1001 merged commit f863a5e into RfidResearchGroup:master Feb 3, 2025
12 checks passed
@Donny-Guo
Copy link
Contributor Author

Yes, I got it. Thanks again @iceman1001. I created a PR for updating the range checking.

@Donny-Guo Donny-Guo deleted the hidbrute branch February 3, 2025 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants