nfc: microread: validate CARD_FOUND event length before parsing targets

[PATCH net] nfc: microread: validate CARD_FOUND event length before parsing targets

Posted by Doruk Tan Ozturk 1 week, 4 days ago

microread_target_discovered() parses a device-supplied MREAD_CARD_FOUND
event into a struct nfc_target, reading fixed offsets and -- for the
ISO-A and ISO-A-3 gates -- a variable-length NFCID1 straight out of the
event skb. The only length check is nfcid1_len vs sizeof(targets->nfcid1);
skb->len itself is never validated, so a short event makes every gate
case read out of bounds past the skb:

  - ISO-A / ISO-A-3: fixed ATQA/SAK/LEN reads plus a memcpy of an
    attacker-controlled nfcid1_len bytes from the NFCID1 offset;
  - ISO-B / NFC-T1 / NFC-T3: a fixed 4- or 8-byte NFCID1 memcpy from a
    fixed offset.

The copied nfcid1 is exported to user space via nfc_targets_found(), so
the over-read is an information leak (and a possible oops on an unmapped
page).

Reject events too short for the fields each gate case reads.

Found by 0sec (https://0sec.ai).

Fixes: cfad1ba87150 ("NFC: Initial support for Inside Secure microread")
Cc: stable@vger.kernel.org
Assisted-by: 0sec
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/nfc/microread/microread.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/microread/microread.c b/drivers/nfc/microread/microread.c
index 4149c5d735bd..9a7a4fd6796b 100644
--- a/drivers/nfc/microread/microread.c
+++ b/drivers/nfc/microread/microread.c
@@ -483,13 +483,19 @@ static void microread_target_discovered(struct nfc_hci_dev *hdev, u8 gate,
 
 	switch (gate) {
 	case MICROREAD_GATE_ID_MREAD_ISO_A:
+		if (skb->len < MICROREAD_EMCF_A_UID) {
+			r = -EINVAL;
+			goto exit_free;
+		}
+
 		targets->supported_protocols =
 		      nfc_hci_sak_to_protocol(skb->data[MICROREAD_EMCF_A_SAK]);
 		targets->sens_res =
 			 be16_to_cpu(*(u16 *)&skb->data[MICROREAD_EMCF_A_ATQA]);
 		targets->sel_res = skb->data[MICROREAD_EMCF_A_SAK];
 		targets->nfcid1_len = skb->data[MICROREAD_EMCF_A_LEN];
-		if (targets->nfcid1_len > sizeof(targets->nfcid1)) {
+		if (targets->nfcid1_len > sizeof(targets->nfcid1) ||
+		    skb->len - MICROREAD_EMCF_A_UID < targets->nfcid1_len) {
 			r = -EINVAL;
 			goto exit_free;
 		}
@@ -497,13 +503,19 @@ static void microread_target_discovered(struct nfc_hci_dev *hdev, u8 gate,
 		       targets->nfcid1_len);
 		break;
 	case MICROREAD_GATE_ID_MREAD_ISO_A_3:
+		if (skb->len < MICROREAD_EMCF_A3_UID) {
+			r = -EINVAL;
+			goto exit_free;
+		}
+
 		targets->supported_protocols =
 		      nfc_hci_sak_to_protocol(skb->data[MICROREAD_EMCF_A3_SAK]);
 		targets->sens_res =
 			 be16_to_cpu(*(u16 *)&skb->data[MICROREAD_EMCF_A3_ATQA]);
 		targets->sel_res = skb->data[MICROREAD_EMCF_A3_SAK];
 		targets->nfcid1_len = skb->data[MICROREAD_EMCF_A3_LEN];
-		if (targets->nfcid1_len > sizeof(targets->nfcid1)) {
+		if (targets->nfcid1_len > sizeof(targets->nfcid1) ||
+		    skb->len - MICROREAD_EMCF_A3_UID < targets->nfcid1_len) {
 			r = -EINVAL;
 			goto exit_free;
 		}
@@ -511,11 +523,21 @@ static void microread_target_discovered(struct nfc_hci_dev *hdev, u8 gate,
 		       targets->nfcid1_len);
 		break;
 	case MICROREAD_GATE_ID_MREAD_ISO_B:
+		if (skb->len < MICROREAD_EMCF_B_UID + 4) {
+			r = -EINVAL;
+			goto exit_free;
+		}
+
 		targets->supported_protocols = NFC_PROTO_ISO14443_B_MASK;
 		memcpy(targets->nfcid1, &skb->data[MICROREAD_EMCF_B_UID], 4);
 		targets->nfcid1_len = 4;
 		break;
 	case MICROREAD_GATE_ID_MREAD_NFC_T1:
+		if (skb->len < MICROREAD_EMCF_T1_UID + 4) {
+			r = -EINVAL;
+			goto exit_free;
+		}
+
 		targets->supported_protocols = NFC_PROTO_JEWEL_MASK;
 		targets->sens_res =
 			le16_to_cpu(*(u16 *)&skb->data[MICROREAD_EMCF_T1_ATQA]);
@@ -523,6 +545,11 @@ static void microread_target_discovered(struct nfc_hci_dev *hdev, u8 gate,
 		targets->nfcid1_len = 4;
 		break;
 	case MICROREAD_GATE_ID_MREAD_NFC_T3:
+		if (skb->len < MICROREAD_EMCF_T3_UID + 8) {
+			r = -EINVAL;
+			goto exit_free;
+		}
+
 		targets->supported_protocols = NFC_PROTO_FELICA_MASK;
 		memcpy(targets->nfcid1, &skb->data[MICROREAD_EMCF_T3_UID], 8);
 		targets->nfcid1_len = 8;
-- 
2.43.0

Re: [PATCH net] nfc: microread: validate CARD_FOUND event length before parsing targets

Posted by David Heidelberg 6 days, 5 hours ago

On 13/07/2026 23:59, Doruk Tan Ozturk wrote:
> microread_target_discovered() parses a device-supplied MREAD_CARD_FOUND
> event into a struct nfc_target, reading fixed offsets and -- for the
> ISO-A and ISO-A-3 gates -- a variable-length NFCID1 straight out of the
> event skb. The only length check is nfcid1_len vs sizeof(targets->nfcid1);
> skb->len itself is never validated, so a short event makes every gate
> case read out of bounds past the skb:
> 
>    - ISO-A / ISO-A-3: fixed ATQA/SAK/LEN reads plus a memcpy of an
>      attacker-controlled nfcid1_len bytes from the NFCID1 offset;
>    - ISO-B / NFC-T1 / NFC-T3: a fixed 4- or 8-byte NFCID1 memcpy from a
>      fixed offset.
> 
> The copied nfcid1 is exported to user space via nfc_targets_found(), so
> the over-read is an information leak (and a possible oops on an unmapped
> page).
> 
> Reject events too short for the fields each gate case reads.
> 
> Found by 0sec (https://0sec.ai).
> 
> Fixes: cfad1ba87150 ("NFC: Initial support for Inside Secure microread")
> Cc: stable@vger.kernel.org
> Assisted-by: 0sec
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
> ---
>   drivers/nfc/microread/microread.c | 31 +++++++++++++++++++++++++++++--
>   1 file changed, 29 insertions(+), 2 deletions(-)
> 
Hello Doruk,

thanks for the patch, 1. 7. I got the same one from Pengpeng, which needs to 
address Fixes and Cc tag [1].

Feel free to sync together for the future effort to not get duplicated (and 
tokens not wasted ;-) )

Adding Pengpeng to Cc.

David

[1] https://lore.kernel.org/all/20260701053709.45176-1-pengpeng@iscas.ac.cn/