Sort EMV applications by the priority indicator, not the AID - #1874
Open
arpitjain099 wants to merge 1 commit into
Open
Sort EMV applications by the priority indicator, not the AID#1874arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
GetCardTokenAID reads both the AID (tag 0x4F) and the Application Priority Indicator (tag 0x87), validates that the priority node exists and is exactly one byte, and then pushes the first byte of the AID as the sort key. So aidPriorityNode is fetched and length-checked and never dereferenced; grep finds it on only three lines, all of them above the push. The key actually used is the AID's first byte, which on essentially every EMV card is 0xA0, the registered application provider prefix, so every key collapses to the same value and the card's stated priorities never enter the sort. EMV Book 1 section 12.4 makes the priority indicator the thing that orders candidate applications. On a multi-application card the keyfile is currently derived from a different application than the card designates. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EMVCard::GetCardTokenAIDreads both the AID (tag0x4F) and the Application Priority Indicator (tag0x87), validates that the priority node exists and is exactly one byte, and then sorts on the wrong one:The length check on
aidPriorityNodeonly makes sense if byte 0 is about to be read, and it never is:grep -rn "aidPriorityNode" src/returns exactly the three lines above, so the value is discarded everywhere in the tree. The key that gets pushed is the AID's first byte, which on essentially every EMV card is0xA0, the registered application provider prefix. Every key collapses to the same value and the ordering degenerates.Verification drives the unmodified
src/Common/TLVParser.cpp, compiled standalone against a small shim forPlatform/PlatformBase.handTcdefs.h, with the selection loop transcribed fromEMVCard.cpplines 280-305. Source identity confirmed by sha256 before building. The input is one PSE READ RECORD response holding two application templates, Visa at priority0x02and Mastercard at priority0x01, so the card ranks Mastercard first:The
sort keys used: 0xA0 0xA0line is the whole finding. EMV Book 1 section 12.4 makes the priority indicator the field that orders candidate applications.On severity, so it is not overread: this is a correctness and spec-conformance bug, not a security issue. A hostile card already supplies every certificate byte that becomes the keyfile, so controlling the ordering gains it nothing, and the wrong selection reaches only the user who inserted the card. What it costs is keyfile determinism on a multi-application card, which is the contract of the feature.