Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

LFRFID: Guard GProxII Wiegand Check Against False Positive and Correct 36-bit Parsing #3868

Merged
merged 11 commits into from
Sep 8, 2024
Merged

LFRFID: Guard GProxII Wiegand Check Against False Positive and Correct 36-bit Parsing #3868

merged 11 commits into from
Sep 8, 2024

Conversation

zinongli
Copy link
Contributor

@zinongli zinongli commented Sep 3, 2024 •
edited by skotopes
Loading

What's new

Verification

  • Try it with these two dumps before and after this PR. Both are supposed to be EM4100:
    test_dumps.zip

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

@zinongli
Copy link
Contributor Author

zinongli commented Sep 3, 2024 •
edited
Loading

Update and explanation for my commit b3831c5: The previous version parsed 36-bit format incorrectly. The original author seems to have interpreted 36-bit as meaning the "profile" being 36 bits long. But here and here we can see it acutally meant it's a 36-bit wiegand. This can also be referenced from card info's record of Chubb 36-bit.

To verify, use PM3 and write a T5577 with this command: lf gproxii clone --xor 236 --fmt 36 --fc 8 --cn 22744
If you are on the version before this PR, it will read as FC: 128, which is wrong. And it actually fails wiegand but because the original code didn't check for wiegand it wouldn't be tested. But with my latest commit FC will read correctly and wiegand check will pass.

@zinongli zinongli changed the title LFRFID: Fix Guard GProxII False Positive LFRFID: Fix Guard GProxII False Positive and 36-bit Parsing Sep 3, 2024
@zinongli zinongli mentioned this pull request Sep 3, 2024
Merged
3 tasks
@zinongli zinongli marked this pull request as draft September 3, 2024 08:56
@zinongli
Copy link
Contributor Author

zinongli commented Sep 4, 2024

Additional changes finished. Waiting for #3869 to be merged first to undraft.

@zinongli zinongli marked this pull request as ready for review September 7, 2024 18:40
@zinongli zinongli changed the title LFRFID: Fix Guard GProxII False Positive and 36-bit Parsing LFRFID: Guard GProxII Wiegand Check Against False Positive and Correct 36-bit Parsing Sep 7, 2024
@skotopes skotopes merged commit a122ee7 into flipperdevices:dev Sep 8, 2024
11 checks passed
ofabel pushed a commit to ofabel/flipperzero-firmware that referenced this pull request Sep 26, 2024
@zinongli @skotopes @ofabel
LFRFID: Guard GProxII Wiegand Check Against False Positive and Correc…
a03cda3 
…t 36-bit Parsing (flipperdevices#3868)

* Update protocol_gproxii.c
* 36 bit format parsing fix
* Update protocol_gproxii.c
* wiegand checks as single function
* LfRfid: simplify gprox wiegand payload validation flow
* LfRfid: extra furi_check in gprox wiegand validation code

Co-authored-by: Aleksandr Kutuzov <alleteam@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@skotopes skotopes skotopes approved these changes

@DrZlo13 DrZlo13 Awaiting requested review from DrZlo13 DrZlo13 is a code owner

@hedger hedger Awaiting requested review from hedger hedger is a code owner

@gsurkov gsurkov Awaiting requested review from gsurkov gsurkov is a code owner

@nminaylov nminaylov Awaiting requested review from nminaylov nminaylov is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zinongli @skotopes