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

"Tianchu Chen" <[email protected]>
Newsgroups dev.linux.lists.oe-linux-nfc,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
From: Tianchu Chen <[email protected]>

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 reject a device-reported length that does
not fit into the caller's buffer before issuing the second SPI
transfer. Also modify callers to pass their real buffer sizes.

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes: cab47333f0f75 ("NFC: Add STMicroelectronics ST95HF driver")
Cc: [email protected]
Signed-off-by: Tianchu Chen <[email protected]>
---
 drivers/nfc/st95hf/core.c | 6 ++++--
 drivers/nfc/st95hf/spi.c  | 7 ++++++-
 drivers/nfc/st95hf/spi.h  | 2 +-
 3 files changed, 11 insertions(+), 4 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..f48abd09a08fc 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,6 +106,11 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
 	else
 		len += receivebuff[1];
 
+	if (len > buff_len) {
+		mutex_unlock(&spicontext->spi_lock);
+		return -E2BIG;
+	}
+
 	/* Now make a transfer to read only relevant bytes */
 	tx_takedata.rx_buf = &receivebuff[2];
 	tx_takedata.len = len - 2;
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.