[PATCH 3/3] smb: client: reject short responses in SMBQueryInformation()

[Diego Oliva]

SMBQueryInformation() reads attr, last_write_time and size out of the
QUERY_INFORMATION_RSP returned by the server without first checking
that a whole QUERY_INFORMATION_RSP was actually received. The length
of the response is recorded in bytes_returned, but nothing constrains
it to be at least sizeof(QUERY_INFORMATION_RSP) before those fields
are dereferenced.

A malicious or compromised SMB1 server can return a reply as short as
35 bytes, a header carrying WordCount and ByteCount of zero, which
checkSMB() accepts because the calculated size matches the length
received. last_write_time and size then lie beyond the received data.
smb_init() hands out a single allocation for both the request and the
response, so those fields are read back out of the request that was
just sent, and a timestamp and file size synthesised from the path
name end up in the inode and are reported by stat(). 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(QUERY_INFORMATION_RSP) bytes long. This cannot reject a
conforming server: the response is documented as wct = 10, so the
smallest valid reply is sizeof(struct smb_hdr) + 2 * 10 + 2, which is
sizeof(QUERY_INFORMATION_RSP).

Fixes: 6b8edfe0f918 ("[CIFS] Support for mounting to older servers part 2.  Add support for legacy getattr (lookup).")
Cc: <stable at vger.kernel.org> # 6.19.x
Assisted-by: Bynario AI
Signed-off-by: Diego Oliva <diego at bynar.io>
---
 fs/smb/client/cifssmb.c | 6 ++++++
 fs/smb/client/trace.h   | 1 +
 2 files changed, 7 insertions(+)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 30b9621664e8..aad1c866d8e1 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -4068,6 +4068,12 @@ SMBQueryInformation(const unsigned int xid, struct cifs_tcon *tcon,
 			 (struct smb_hdr *) pSMBr, &bytes_returned, 0);
 	if (rc) {
 		cifs_dbg(FYI, "Send error in QueryInfo = %d\n", rc);
+	} else if (bytes_returned < (int)sizeof(QUERY_INFORMATION_RSP)) {
+		/* check that the received response can hold a whole rsp */
+		cifs_dbg(FYI, "%s: server returned short header. got=%d expected=%zu\n",
+			 __func__, bytes_returned, sizeof(QUERY_INFORMATION_RSP));
+		rc = smb_EIO2(smb_eio_trace_qinfo_rsp_short,
+			      bytes_returned, sizeof(QUERY_INFORMATION_RSP));
 	} else if (data) {
 		struct timespec64 ts;
 		__u32 time = le32_to_cpu(pSMBr->last_write_time);
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index a1c0cb1a5833..6926a62ddb05 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -71,6 +71,7 @@
 	EM(smb_eio_trace_qfsinfo_bcc_too_small,		"qfsinfo_bcc_too_small") \
 	EM(smb_eio_trace_qfsposixinfo_bcc_too_small,	"qfsposixinfo_bcc_too_small") \
 	EM(smb_eio_trace_qfsunixinfo_bcc_too_small,	"qfsunixinfo_bcc_too_small") \
+	EM(smb_eio_trace_qinfo_rsp_short,		"qinfo_rsp_short") \
 	EM(smb_eio_trace_qpathinfo_bcc_too_small,	"qpathinfo_bcc_too_small") \
 	EM(smb_eio_trace_qpathinfo_invalid,		"qpathinfo_invalid") \
 	EM(smb_eio_trace_qreparse_data_area,		"qreparse_data_area") \
-- 
2.39.5