[PATCH v3] nfc: st95hf: validate device-reported response length before reading

[PATCH v3] nfc: st95hf: validate device-reported response length before reading

[Tianchu Chen]

From: Tianchu Chen <flynnnchen@tencent.com>

st95hf_spi_recv_response() reads a 2-byte header from the device,
derives the total response length from it (up to 1025 bytes via the
long-frame encoding) and then blindly reads len - 2 more bytes into
the caller's buffer, whose size it knows nothing about.

Call sites can be overflowed by a device reporting a large length,
for example, st95hf_irq_thread_handler() passes the data area of a
280-byte skb.

ST95HF datasheet states "In Reader mode it is possible to receive up to
528 bytes of frame data from VICC and TypeB cards", so technically a bogus
NFC tag may trigger the OOB-write if the card-reader's firmware allows
large packets.

Add a buff_len parameter and clamp the device-reported length that does
not fit into the caller's buffer, Also modify callers to pass their
real buffer sizes.

Note that the clamped transfer must still run rather than bailing
out early: the header transfer keeps chip select asserted via
cs_change, and only completing a follow-up message lets the SPI
core deassert it. Simply discards the unread bytes, then report the
overflow with -E2BIG after the transfer completes.

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes: cab47333f0f75 ("NFC: Add STMicroelectronics ST95HF driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
v3: resend as a separate thread; no code changes (v2 was sent as an
in-thread follow-up by mistake).

v2: clamp the payload read to the caller's buffer and let the second
SPI transfer complete instead of returning early, so chip select (kept
asserted by the header transfer's cs_change) is deasserted by the SPI
core; report the overflow with -E2BIG after the transfer completes.

 drivers/nfc/st95hf/core.c |  6 ++++--
 drivers/nfc/st95hf/spi.c  | 16 +++++++++++++---
 drivers/nfc/st95hf/spi.h  |  2 +-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 4d772a308bfff..0fbb60deada23 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -283,7 +283,8 @@ static int st95hf_send_recv_cmd(struct st95hf_context *st95context,
 		unsigned char st95hf_response_arr[2];
 
 		ret = st95hf_spi_recv_response(&st95context->spicontext,
-					       st95hf_response_arr);
+					       st95hf_response_arr,
+					       sizeof(st95hf_response_arr));
 		if (ret < 0) {
 			dev_err(dev, "spi error from st95hf_spi_recv_response(), err = 0x%x\n",
 				ret);
@@ -800,7 +801,8 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
 
 	mutex_lock(&stcontext->rm_lock);
 	res_len = st95hf_spi_recv_response(&stcontext->spicontext,
-					   skb_resp->data);
+					   skb_resp->data,
+					   skb_tailroom(skb_resp));
 	if (res_len < 0) {
 		dev_err(spidevice, "TISR spi response err = 0x%x\n", res_len);
 		result = res_len;
diff --git a/drivers/nfc/st95hf/spi.c b/drivers/nfc/st95hf/spi.c
index ffaf2789c4069..a69f7c5e3695e 100644
--- a/drivers/nfc/st95hf/spi.c
+++ b/drivers/nfc/st95hf/spi.c
@@ -66,7 +66,7 @@ EXPORT_SYMBOL_GPL(st95hf_spi_send);
 
 /* Function to Receive command Response */
 int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
-			     unsigned char *receivebuff)
+			     unsigned char *receivebuff, int buff_len)
 {
 	int len = 0;
 	struct spi_transfer tx_takedata;
@@ -106,9 +106,16 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
 	else
 		len += receivebuff[1];
 
-	/* Now make a transfer to read only relevant bytes */
+	/*
+	 * Now make a transfer to read only relevant bytes, clamped to
+	 * the caller's buffer.  The transfer must run even when the
+	 * device overreports the length: the header transfer above kept
+	 * chip select asserted via cs_change, and only completing this
+	 * message lets the SPI core deassert it.  The device discards
+	 * the unread bytes at the chip select rising edge.
+	 */
 	tx_takedata.rx_buf = &receivebuff[2];
-	tx_takedata.len = len - 2;
+	tx_takedata.len = min(len, buff_len) - 2;
 
 	spi_message_init(&m);
 	spi_message_add_tail(&tx_takedata, &m);
@@ -122,6 +129,9 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
 		return ret;
 	}
 
+	if (len > buff_len)
+		return -E2BIG;
+
 	return len;
 }
 EXPORT_SYMBOL_GPL(st95hf_spi_recv_response);
diff --git a/drivers/nfc/st95hf/spi.h b/drivers/nfc/st95hf/spi.h
index 3ab678734c174..443a5053c0128 100644
--- a/drivers/nfc/st95hf/spi.h
+++ b/drivers/nfc/st95hf/spi.h
@@ -45,7 +45,7 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
 		    enum req_type reqtype);
 
 int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
-			     unsigned char *receivebuff);
+			     unsigned char *receivebuff, int buff_len);
 
 int st95hf_spi_recv_echo_res(struct st95hf_spi_context *spicontext,
 			     unsigned char *receivebuff);
-- 
2.51.0