NFC: Fix stack buffer overflows in MFUL FAST_READ and DESFire file settings parsers#4369
Open
qp-x-qp wants to merge 1 commit into
Open
NFC: Fix stack buffer overflows in MFUL FAST_READ and DESFire file settings parsers#4369qp-x-qp wants to merge 1 commit into
qp-x-qp wants to merge 1 commit into
Conversation
…ttings parsers mf_ultralight_listener.c: FAST_READ handler wrote page_cnt*4 bytes into a fixed pages[64] stack buffer without checking page_cnt <= 64. For tags with pages_total > 64 (NTAG215: 135, NTAG216: 231) a reader could trigger an overflow of up to 668 bytes. Fix: reject requests where page_cnt exceeds COUNT_OF(pages) with NAK. mf_desfire_i.c: additional_access_rights_len (a count) was validated against MF_DESFIRE_MAX_KEYS * sizeof(MfDesfireFileAccessRights) (= 28) instead of MF_DESFIRE_MAX_KEYS - 1 (= 13), allowing a rogue tag to write up to 56 bytes into a 26-byte stack slot. Fix: compare count against MF_DESFIRE_MAX_KEYS - 1.
qp-x-qp
requested review from
DrZlo13,
gornekich,
gsurkov and
hedger
as code owners
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
2 participants
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.
mf_ultralight_listener.c:171 — FAST_READ overflow (emulation path)
FAST_READ handler copies
page_cnt * 4bytes into a fixedpages[64]stack buffer (256 bytes) without validating
page_cnt <= 64. For any tagwith
pages_total > 64(NTAG215: 135 pages, NTAG216: 231 pages), a readersending FAST_READ with
end_page = pages_total - 1passes the existingbounds check and triggers an overflow of up to 668 bytes of stack memory.
Fix: reject with NAK if
page_cnt > COUNT_OF(pages).mf_desfire_i.c:278 — access_rights overflow (reader path)
additional_access_rights_len(a count of entries) was checked againstMF_DESFIRE_MAX_KEYS * sizeof(MfDesfireFileAccessRights)(= 28) insteadof
MF_DESFIRE_MAX_KEYS - 1(= 13). A rogue DESFire tag returningadditional_access_rights_len = 28passes the check but triggers amemcpyof 56 bytes into a 26-byte stack slot (access_rights[1..13]),overflowing by 30 bytes of tag-controlled data.
Fix: compare count against
MF_DESFIRE_MAX_KEYS - 1.reader sends end_page=0xE6 — device hard-faults before fix, NAKs after)