From 44dfdf6c245622fc74c5f1941fd1900ac24734e3 Mon Sep 17 00:00:00 2001

From: David Disseldorp <ddiss@suse.de>

Date: Fri, 7 Apr 2023 00:34:11 +0200

Subject: [PATCH] cifs: fix negotiate context parsing

Git-commit: 5105a7ffce19160e7062aee67fb6b3b8a1b56d78

Patch-mainline: v6.3-rc7

References: bsc#1210301

smb311_decode_neg_context() doesn't properly check against SMB packet

boundaries prior to accessing individual negotiate context entries. This

is due to the length check omitting the eight byte smb2_neg_context

header, as well as incorrect decrementing of len_of_ctxts.

Fixes: 5100d8a3fe03 ("SMB311: Improve checking of negotiate security contexts")

Reported-by: Volker Lendecke <vl@samba.org>

Reviewed-by: Paulo Alcantara (SUSE) <pc@manguebit.com>

Signed-off-by: David Disseldorp <ddiss@suse.de>

Signed-off-by: Steve French <stfrench@microsoft.com>

[ddiss: rebase against 5.3 without d7173623bf0b15]

---

 fs/cifs/smb2pdu.c | 41 +++++++++++++++++++++++++++++++----------

 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c

index 7ca02350cbfc2..ac67615308e88 100644

--- a/fs/cifs/smb2pdu.c

+++ b/fs/cifs/smb2pdu.c

@@ -585,11 +585,15 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,

 }

+/* If invalid preauth context warn but use what we requested, SHA-512 */

 static void decode_preauth_context(struct smb2_preauth_neg_context *ctxt)

 {

 	unsigned int len = le16_to_cpu(ctxt->DataLength);

-	/* If invalid preauth context warn but use what we requested, SHA-512 */

+	/*

+	 * Caller checked that DataLength remains within SMB boundary. We still

+	 * need to confirm that one HashAlgorithms member is accounted for.

+	 */

 	if (len < MIN_PREAUTH_CTXT_DATA_LEN) {

 		pr_warn_once("server sent bad preauth context\n");

 		return;

@@ -608,7 +612,11 @@ static void decode_compress_ctx(struct TCP_Server_Info *server,

 {

 	unsigned int len = le16_to_cpu(ctxt->DataLength);

-	/* sizeof compress context is a one element compression capbility struct */

+	/*

+	 * Caller checked that DataLength remains within SMB boundary. We still

+	 * need to confirm that one CompressionAlgorithms member is accounted

+	 * for.

+	 */

 	if (len < 10) {

 		pr_warn_once("server sent bad compression cntxt\n");

 		return;

@@ -630,6 +638,11 @@ static int decode_encrypt_ctx(struct TCP_Server_Info *server,

 	unsigned int len = le16_to_cpu(ctxt->DataLength);

 	cifs_dbg(FYI, "decode SMB3.11 encryption neg context of len %d\n", len);

+	/*

+	 * Caller checked that DataLength remains within SMB boundary. We still

+	 * need to confirm that one Cipher flexible array member is accounted

+	 * for.

+	 */

 	if (len < MIN_ENCRYPT_CTXT_DATA_LEN) {

 		pr_warn_once("server sent bad crypto ctxt len\n");

 		return -EINVAL;

@@ -676,6 +689,11 @@ static void decode_signing_ctx(struct TCP_Server_Info *server,

 {

 	unsigned int len = le16_to_cpu(pctxt->DataLength);

+	/*

+	 * Caller checked that DataLength remains within SMB boundary. We still

+	 * need to confirm that one SigningAlgorithms flexible array member is

+	 * accounted for.

+	 */

 	if ((len < 4) || (len > 16)) {

 		pr_warn_once("server sent bad signing negcontext\n");

 		return;

@@ -717,14 +735,19 @@ static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp,

 	for (i = 0; i < ctxt_cnt; i++) {

 		int clen;

 		/* check that offset is not beyond end of SMB */

-		if (len_of_ctxts == 0)

-			break;

-

 		if (len_of_ctxts < sizeof(struct smb2_neg_context))

 			break;

 		pctx = (struct smb2_neg_context *)(offset + (char *)rsp);

-		clen = le16_to_cpu(pctx->DataLength);

+		clen = sizeof(struct smb2_neg_context)

+			+ le16_to_cpu(pctx->DataLength);

+		/*

+		 * 2.2.4 SMB2 NEGOTIATE Response

+		 * Subsequent negotiate contexts MUST appear at the first 8-byte

+		 * aligned offset following the previous negotiate context.

+		 */

+		if (i + 1 != ctxt_cnt)

+			clen = ALIGN(clen, 8);

 		if (clen > len_of_ctxts)

 			break;

@@ -745,12 +768,10 @@ static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp,

 		else

 			cifs_server_dbg(VFS, "unknown negcontext of type %d ignored\n",

 				le16_to_cpu(pctx->ContextType));

-

 		if (rc)

 			break;

-		/* offsets must be 8 byte aligned */

-		clen = (clen + 7) & ~0x7;

-		offset += clen + sizeof(struct smb2_neg_context);

+

+		offset += clen;

 		len_of_ctxts -= clen;

 	}

 	return rc;

-- 

2.40.0