[PATCH 1/3] smb: client: reject short WRITE responses in the SMB1 write paths

[Diego Oliva]

cifs_writev_callback(), CIFSSMBWrite() and CIFSSMBWrite2() all read
Count and CountHigh out of the WRITE_RSP returned by the server
without first checking that a whole WRITE_RSP was actually received.
The length of the response is available to each of them, in
mid->response_pdu_len, bytes_returned and rsp_iov.iov_len
respectively, but none of them constrains it to be at least
sizeof(WRITE_RSP) before those fields are dereferenced. In the
asynchronous case cifs_check_receive() has run first, but it only
verifies the signature and maps the SMB error; it performs no length
validation.

A malicious or compromised SMB1 server can therefore return a response
shorter than the WRITE_RSP header and still have it parsed. In
CIFSSMBWrite() the response buffer is the request buffer, since
smb_init() hands out a single allocation for both, so the count is
read back out of the request that was just sent; in the other two the
reply lives in the demultiplex thread's buffer, so it comes from
recycled slab memory. Either way the client reports a number of bytes
written that the server never sent. SMB1 is not negotiated by default;
reaching this code requires an explicit vers=1.0 mount.

Reject the response unless it is at least sizeof(WRITE_RSP) bytes
long. This cannot reject a conforming server: WRITE_RSP is documented
as wct = 6, so the smallest valid reply is
sizeof(struct smb_hdr) + 2 * 6 + 2, which is sizeof(WRITE_RSP).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: c28c89fc43e3 ("cifs: add cifs_async_writev")
Cc: <stable@xxxxxxxxxxxxxxx> # 6.19.x
Assisted-by: Bynario AI
Signed-off-by: Diego Oliva <diego@xxxxxxxx>
---
 fs/smb/client/cifssmb.c | 21 +++++++++++++++++++++
 fs/smb/client/trace.h   |  1 +
 2 files changed, 22 insertions(+)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index f9aff0712794..b1525d491ce5 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1880,6 +1880,12 @@ CIFSSMBWrite(const unsigned int xid, struct cifs_io_parms *io_parms,
 	cifs_stats_inc(&tcon->stats.cifs_stats.num_writes);
 	if (rc) {
 		cifs_dbg(FYI, "Send error in write = %d\n", rc);
+	} else if (bytes_returned < (int)sizeof(WRITE_RSP)) {
+		/* check that the received response can hold a whole WRITE_RSP */
+		cifs_dbg(FYI, "%s: server returned short header. got=%d expected=%zu\n",
+			 __func__, bytes_returned, sizeof(WRITE_RSP));
+		rc = smb_EIO2(smb_eio_trace_write_rsp_short,
+			      bytes_returned, sizeof(WRITE_RSP));
 	} else {
 		*nbytes = le16_to_cpu(pSMBr->CountHigh);
 		*nbytes = (*nbytes) << 16;
@@ -1927,6 +1933,15 @@ cifs_writev_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 		if (result != 0)
 			break;
 
+		if (mid->response_pdu_len < sizeof(WRITE_RSP)) {
+			/* check that the received response can hold a whole WRITE_RSP */
+			cifs_dbg(FYI, "%s: server returned short header. got=%u expected=%zu\n",
+				 __func__, mid->response_pdu_len, sizeof(WRITE_RSP));
+			result = smb_EIO2(smb_eio_trace_write_rsp_short,
+					  mid->response_pdu_len, sizeof(WRITE_RSP));
+			break;
+		}
+
 		written = le16_to_cpu(smb->CountHigh);
 		written <<= 16;
 		written += le16_to_cpu(smb->Count);
@@ -2150,6 +2165,12 @@ CIFSSMBWrite2(const unsigned int xid, struct cifs_io_parms *io_parms,
 	} else if (resp_buf_type == 0) {
 		/* presumably this can not happen, but best to be safe */
 		rc = smb_EIO1(smb_eio_trace_write_bad_buf_type, resp_buf_type);
+	} else if (rsp_iov.iov_len < sizeof(WRITE_RSP)) {
+		/* check that the received response can hold a whole WRITE_RSP */
+		cifs_dbg(FYI, "%s: server returned short header. got=%zu expected=%zu\n",
+			 __func__, rsp_iov.iov_len, sizeof(WRITE_RSP));
+		rc = smb_EIO2(smb_eio_trace_write_rsp_short,
+			      rsp_iov.iov_len, sizeof(WRITE_RSP));
 	} else {
 		WRITE_RSP *pSMBr = (WRITE_RSP *)rsp_iov.iov_base;
 		*nbytes = le16_to_cpu(pSMBr->CountHigh);
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index b442cccd1530..a0ad8068425e 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -150,6 +150,7 @@
 	EM(smb_eio_trace_write_bad_buf_type,		"write_bad_buf_type") \
 	EM(smb_eio_trace_write_mid_state_unknown,	"write_mid_state_unknown") \
 	EM(smb_eio_trace_write_rsp_malformed,		"write_rsp_malformed") \
+	EM(smb_eio_trace_write_rsp_short,		"write_rsp_short") \
 	E_(smb_eio_trace_write_too_far,			"write_too_far")
 
 #define smb3_rw_credits_traces \
-- 
2.39.5