[PATCH 1/2] nfc: pn533: bound device-supplied lengths in the receive path

[Michael Bommarito <michael.bommarito@gmail.com>]

The PN533 receive path trusts device-supplied length fields and reads
past the received buffer when they exceed it. The rx skb is sized to the
device's USB transfer length, so a malicious PN53x reader triggers
out-of-bounds reads in the standard/ACR122 frame validators, in Type A
target parsing (nfcid_len), and in the autopoll record walk.

Thread the received length into the rx_is_frame_valid callbacks, reject
ACR122 frames with datalen < 2, and add the missing nfcid and per-record
bounds.

This was found with static analysis and confirmed with the KUnit tests
added in the following patch.

Fixes: c46ee38620a2 ("NFC: pn533: add NXP pn533 nfc device driver")
Fixes: 9815c7cf22da ("NFC: pn533: Separate physical layer from the core implementation")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/nfc/pn533/pn533.c | 32 ++++++++++++++++++++++++++++++--
 drivers/nfc/pn533/pn533.h |  3 ++-
 drivers/nfc/pn533/usb.c   | 12 ++++++++++--
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index d7bdbc82e2ba2..984a61a83feaf 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -274,11 +274,15 @@ static void pn533_std_tx_update_payload_len(void *_frame, int len)
 	frame->datalen += len;
 }
 
-static bool pn533_std_rx_frame_is_valid(void *_frame, struct pn533 *dev)
+static bool pn533_std_rx_frame_is_valid(void *_frame, size_t frame_len,
+					struct pn533 *dev)
 {
 	u8 checksum;
 	struct pn533_std_frame *stdf = _frame;
 
+	if (frame_len < sizeof(struct pn533_std_frame))
+		return false;
+
 	if (stdf->start_frame != cpu_to_be16(PN533_STD_FRAME_SOF))
 		return false;
 
@@ -286,6 +290,10 @@ static bool pn533_std_rx_frame_is_valid(void *_frame, struct pn533 *dev)
 		/* Standard frame code */
 		dev->ops->rx_header_len = PN533_STD_FRAME_HEADER_LEN;
 
+		if (frame_len < sizeof(struct pn533_std_frame) +
+				stdf->datalen + PN533_STD_FRAME_TAIL_LEN)
+			return false;
+
 		checksum = pn533_std_checksum(stdf->datalen);
 		if (checksum != stdf->datalen_checksum)
 			return false;
@@ -299,6 +307,11 @@ static bool pn533_std_rx_frame_is_valid(void *_frame, struct pn533 *dev)
 
 		dev->ops->rx_header_len = PN533_EXT_FRAME_HEADER_LEN;
 
+		if (frame_len < sizeof(struct pn533_ext_frame) ||
+		    frame_len < sizeof(struct pn533_ext_frame) +
+				be16_to_cpu(eif->datalen) + PN533_STD_FRAME_TAIL_LEN)
+			return false;
+
 		checksum = pn533_ext_checksum(be16_to_cpu(eif->datalen));
 		if (checksum != eif->datalen_checksum)
 			return false;
@@ -701,6 +714,10 @@ static bool pn533_target_type_a_is_valid(struct pn533_target_type_a *type_a,
 	if (type_a->nfcid_len > NFC_NFCID1_MAXSIZE)
 		return false;
 
+	if (sizeof(struct pn533_target_type_a) + type_a->nfcid_len >
+	    (size_t)target_data_len)
+		return false;
+
 	return true;
 }
 
@@ -1399,6 +1416,7 @@ static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
 {
 	struct pn532_autopoll_resp *apr;
 	struct nfc_target nfc_tgt;
+	size_t remaining;
 	u8 nbtg;
 	int rc;
 
@@ -1417,12 +1435,21 @@ static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
 		goto stop_poll;
 	}
 
+	if (resp->len < 1)
+		return -EAGAIN;
+
 	nbtg = resp->data[0];
 	if ((nbtg > 2) || (nbtg <= 0))
 		return -EAGAIN;
 
+	remaining = resp->len - 1;
 	apr = (struct pn532_autopoll_resp *)&resp->data[1];
 	while (nbtg--) {
+		if (remaining < sizeof(struct pn532_autopoll_resp) ||
+		    apr->ln < 1 ||
+		    remaining < (size_t)apr->ln + 2)
+			return -EAGAIN;
+
 		memset(&nfc_tgt, 0, sizeof(struct nfc_target));
 		switch (apr->type) {
 		case PN532_AUTOPOLL_TYPE_ISOA:
@@ -1468,6 +1495,7 @@ static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
 		}
 
 		dev->tgt_available_prots = nfc_tgt.supported_protocols;
+		remaining -= (size_t)apr->ln + 2;
 		apr = (struct pn532_autopoll_resp *)
 			(apr->tgdata + (apr->ln - 1));
 	}
@@ -2189,7 +2217,7 @@ void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status)
 	print_hex_dump_debug("PN533 RX: ", DUMP_PREFIX_NONE, 16, 1, skb->data,
 			     dev->ops->rx_frame_size(skb->data), false);
 
-	if (!dev->ops->rx_is_frame_valid(skb->data, dev)) {
+	if (!dev->ops->rx_is_frame_valid(skb->data, skb->len, dev)) {
 		nfc_err(dev->dev, "Received an invalid frame\n");
 		dev->cmd->status = -EIO;
 	} else if (!pn533_rx_frame_is_cmd_response(dev, skb->data)) {
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 09e35b8693f5a..08804f88b38a3 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -201,7 +201,8 @@ struct pn533_frame_ops {
 	int tx_header_len;
 	int tx_tail_len;
 
-	bool (*rx_is_frame_valid)(void *frame, struct pn533 *dev);
+	bool (*rx_is_frame_valid)(void *frame, size_t frame_len,
+				  struct pn533 *dev);
 	bool (*rx_frame_is_ack)(void *frame);
 	int (*rx_frame_size)(void *frame);
 	int rx_header_len;
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index efb07f944fce2..1d50600609dd4 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -312,14 +312,22 @@ static void pn533_acr122_tx_update_payload_len(void *_frame, int len)
 	frame->datalen += len;
 }
 
-static bool pn533_acr122_is_rx_frame_valid(void *_frame, struct pn533 *dev)
+static bool pn533_acr122_is_rx_frame_valid(void *_frame, size_t frame_len,
+					   struct pn533 *dev)
 {
 	struct pn533_acr122_rx_frame *frame = _frame;
 
+	if (frame_len < sizeof(struct pn533_acr122_rx_frame))
+		return false;
+
 	if (frame->ccid.type != 0x83)
 		return false;
 
-	if (!frame->ccid.datalen)
+	if (frame->ccid.datalen < 2)
+		return false;
+
+	if (frame_len < sizeof(struct pn533_acr122_rx_frame) +
+			frame->ccid.datalen)
 		return false;
 
 	if (frame->data[frame->ccid.datalen - 2] == 0x63)
-- 
2.53.0