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

Secure PIN Entry/Modification Fails with PCSC-Lite 1.8.9 and newer #2

Open
virgil382 opened this issue Mar 7, 2014 · 2 comments
Open

Comments

@virgil382
Copy link

In PCSC-Lite 1.8.9, the structure PIN_VERIFY_STRUCTURE defines abData as a zero-size array. So PIN_VERIFY_STRUCTURE ends up being 19 bytes long. In 1.8.8 and earlier, it is defined as a 1-byte array. So PIN_VERIFY_STRUCTURE ends up being 20 bytes long. See https://alioth.debian.org/frs/download.php/latestfile/39/pcsc-lite-1.8.9.tar.bz2 in file pcsc-lite-1.8.9.tar-1/pcsc-lite-1.8.9/src/PCSC/reader.h

This driver's code (e.g. cjeca32/CCIDReader.cpp) assumes the 1-byte array definition, and will not work with 1.8.9 and newer. For example, the following code behave incorrectly when compiled with PCSC-Lite 1.8.9 and newer:
if(InputLength!=sizeof(PIN_VERIFY_STRUCTURE)-sizeof(uint8_t)+((PIN_VERIFY_STRUCTURE *)Input)->ulDataLength) { ...

A proposed fix for this issue is to change the code as follows so that it work correctly with both definitions of PIN_VERIFY_STRUCTURE:
if(InputLength!=sizeof(PIN_VERIFY_STRUCTURE)-sizeof(((PIN_VERIFY_STRUCTURE*)(0))->abData)+((PIN_VERIFY_STRUCTURE *)Input)->ulDataLength) { ...

I.e., the key to this proposed fix is to replace
sizeof(uint8_t)
which always evaluates to 1, with
sizeof(((PIN_VERIFY_STRUCTURE*)(0))->abData)
which evaluates to either 1 or 0 depending on the definition of PIN_VERIFY_STRUCTURE.

@larskanis
Copy link
Owner

Sounds good. Since you seam to have already identified the code lines to change, I would be glad to merge a pull request with your changes!

@virgil382
Copy link
Author

Hi Lars,

Thanks for your reply. Sorry but can't comply for fear of breaking something and for lack of more time to analyze miscellaneous problems that I seem to be having with forking, cloning, and compiling this version on my CentOS system. (The version that worked for me and that I used to analyze this issue is from the ReinerSCT CD entitled pcsc-cyberjack_3.99.5final.SP03.tar.gz.)

In summary, there are 3 files that need changing:
2 lines in doc/verifypin_ascii.c
2 lines in doc/verifypin_fpin2.c
2 lines in cjeca32/CCIDReader.cpp

Please let me know if I can be of more help I will try my best to comply.

Best regards,
--Virgil

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

No branches or pull requests

2 participants