[PATCH v2] nfc: pn533: bounds-check the ATR_RES response before parsing it

[PATCH v2] nfc: pn533: bounds-check the ATR_RES response before parsing it

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

pn533_poll_dep_complete(), pn533_in_dep_link_up_complete() and
pn533_activate_target_nfcdep() cast resp->data to the fixed ATR_RES
header (struct pn533_cmd_jump_dep_response / pn533_cmd_activate_response)
and, on a successful response, read nfcid3t[] and compute the general-
bytes length as resp->len - sizeof(header). A short response leaves that
read past the end of the skb data, an out-of-bounds read; the copied
nfcid3t reaches userspace via nfc_targets_found() and netlink.

Only the success path needs the full fixed header: a no-peer/failure
reply is just the status byte, and the existing failure handling (turn
the radio off and re-arm polling, or print the error) must keep running
for it. So validate resp->len >= sizeof(*rsp) after the status check, on
the success path, in all three handlers, before nfcid3t and the general
bytes are read.

Found by code review; not reproduced.

Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
Changes in v2 (thanks to the Sashiko AI review):
 - move the length check to the SUCCESS path (after the status check), so
   a no-peer/timeout reply (resp->len == 1) still takes the existing
   "turn radio off, re-arm polling" / error path instead of returning
   -EINVAL and stalling the poll loop
 - also guard pn533_activate_target_nfcdep(), the third ATR_RES parser
 - add Fixes: tags for the two commits that introduced the parsing
 - drop the inaccurate claim that the other handlers already length-check
   resp->len (only pn533_init_target_complete() does)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index b0133e51dce9..9c65b52a8a14 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -1319,6 +1319,17 @@ static int pn533_poll_dep_complete(struct pn533 *dev, void *arg,
 		return 0;
 	}
 
+	/*
+	 * A successful ATR_RES carries the full fixed header (nfcid3t[] and
+	 * the rest); the general bytes start at offset sizeof(*rsp). Reject a
+	 * short success reply before reading nfcid3t and computing the
+	 * general-bytes length, to avoid an out-of-bounds read.
+	 */
+	if (resp->len < sizeof(*rsp)) {
+		rc = -EINVAL;
+		goto error;
+	}
+
 	dev_dbg(dev->dev, "Creating new target");
 
 	nfc_target.supported_protocols = NFC_PROTO_NFC_DEP_MASK;
@@ -1799,6 +1810,11 @@ static int pn533_activate_target_nfcdep(struct pn533 *dev)
 		return -EIO;
 	}
 
+	if (resp->len < sizeof(*rsp)) {
+		dev_kfree_skb(resp);
+		return -EIO;
+	}
+
 	/* ATR_RES general bytes are located at offset 16 */
 	gt_len = resp->len - 16;
 	rc = nfc_set_remote_general_bytes(dev->nfc_dev, rsp->gt, gt_len);
@@ -1937,6 +1953,11 @@ static int pn533_in_dep_link_up_complete(struct pn533 *dev, void *arg,
 		goto error;
 	}
 
+	if (resp->len < sizeof(*rsp)) {
+		rc = -EINVAL;
+		goto error;
+	}
+
 	if (!dev->tgt_available_prots) {
 		struct nfc_target nfc_target;
 
-- 
2.43.0