cifs: fix negotiate context parsing

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>

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 2b92132097dc89363f4470b54a3355f61e75109e..4245249dbba86ab247f27f1f75f36348eaa23a4d 100644 (file)
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -587,11 +587,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;
@@ -610,7 +614,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;
@@ -632,6 +640,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;
@@ -678,6 +691,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;
@@ -719,14 +737,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;
 
@@ -747,12 +770,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 = ALIGN(clen, 8);
-               offset += clen + sizeof(struct smb2_neg_context);
+
+               offset += clen;
                len_of_ctxts -= clen;
        }
        return rc;