ksmbd: fix multiple out-of-bounds read during context decoding
authorKuan-Ting Chen <h3xrabbit@gmail.com>
Fri, 19 May 2023 14:00:24 +0000 (23:00 +0900)
committerSteve French <stfrench@microsoft.com>
Sat, 27 May 2023 01:27:46 +0000 (20:27 -0500)
Check the remaining data length before accessing the context structure
to ensure that the entire structure is contained within the packet.
Additionally, since the context data length `ctxt_len` has already been
checked against the total packet length `len_of_ctxts`, update the
comparison to use `ctxt_len`.

Cc: stable@vger.kernel.org
Signed-off-by: Kuan-Ting Chen <h3xrabbit@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
fs/smb/server/smb2pdu.c

index 83c6682..a1939ff 100644 (file)
@@ -845,13 +845,14 @@ static void assemble_neg_contexts(struct ksmbd_conn *conn,
 
 static __le32 decode_preauth_ctxt(struct ksmbd_conn *conn,
                                  struct smb2_preauth_neg_context *pneg_ctxt,
-                                 int len_of_ctxts)
+                                 int ctxt_len)
 {
        /*
         * sizeof(smb2_preauth_neg_context) assumes SMB311_SALT_SIZE Salt,
         * which may not be present. Only check for used HashAlgorithms[1].
         */
-       if (len_of_ctxts < MIN_PREAUTH_CTXT_DATA_LEN)
+       if (ctxt_len <
+           sizeof(struct smb2_neg_context) + MIN_PREAUTH_CTXT_DATA_LEN)
                return STATUS_INVALID_PARAMETER;
 
        if (pneg_ctxt->HashAlgorithms != SMB2_PREAUTH_INTEGRITY_SHA512)
@@ -863,15 +864,23 @@ static __le32 decode_preauth_ctxt(struct ksmbd_conn *conn,
 
 static void decode_encrypt_ctxt(struct ksmbd_conn *conn,
                                struct smb2_encryption_neg_context *pneg_ctxt,
-                               int len_of_ctxts)
+                               int ctxt_len)
 {
-       int cph_cnt = le16_to_cpu(pneg_ctxt->CipherCount);
-       int i, cphs_size = cph_cnt * sizeof(__le16);
+       int cph_cnt;
+       int i, cphs_size;
+
+       if (sizeof(struct smb2_encryption_neg_context) > ctxt_len) {
+               pr_err("Invalid SMB2_ENCRYPTION_CAPABILITIES context size\n");
+               return;
+       }
 
        conn->cipher_type = 0;
 
+       cph_cnt = le16_to_cpu(pneg_ctxt->CipherCount);
+       cphs_size = cph_cnt * sizeof(__le16);
+
        if (sizeof(struct smb2_encryption_neg_context) + cphs_size >
-           len_of_ctxts) {
+           ctxt_len) {
                pr_err("Invalid cipher count(%d)\n", cph_cnt);
                return;
        }
@@ -919,15 +928,22 @@ static void decode_compress_ctxt(struct ksmbd_conn *conn,
 
 static void decode_sign_cap_ctxt(struct ksmbd_conn *conn,
                                 struct smb2_signing_capabilities *pneg_ctxt,
-                                int len_of_ctxts)
+                                int ctxt_len)
 {
-       int sign_algo_cnt = le16_to_cpu(pneg_ctxt->SigningAlgorithmCount);
-       int i, sign_alos_size = sign_algo_cnt * sizeof(__le16);
+       int sign_algo_cnt;
+       int i, sign_alos_size;
+
+       if (sizeof(struct smb2_signing_capabilities) > ctxt_len) {
+               pr_err("Invalid SMB2_SIGNING_CAPABILITIES context length\n");
+               return;
+       }
 
        conn->signing_negotiated = false;
+       sign_algo_cnt = le16_to_cpu(pneg_ctxt->SigningAlgorithmCount);
+       sign_alos_size = sign_algo_cnt * sizeof(__le16);
 
        if (sizeof(struct smb2_signing_capabilities) + sign_alos_size >
-           len_of_ctxts) {
+           ctxt_len) {
                pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt);
                return;
        }
@@ -965,18 +981,16 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
        len_of_ctxts = len_of_smb - offset;
 
        while (i++ < neg_ctxt_cnt) {
-               int clen;
-
-               /* check that offset is not beyond end of SMB */
-               if (len_of_ctxts == 0)
-                       break;
+               int clen, ctxt_len;
 
                if (len_of_ctxts < sizeof(struct smb2_neg_context))
                        break;
 
                pctx = (struct smb2_neg_context *)((char *)pctx + offset);
                clen = le16_to_cpu(pctx->DataLength);
-               if (clen + sizeof(struct smb2_neg_context) > len_of_ctxts)
+               ctxt_len = clen + sizeof(struct smb2_neg_context);
+
+               if (ctxt_len > len_of_ctxts)
                        break;
 
                if (pctx->ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) {
@@ -987,7 +1001,7 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
 
                        status = decode_preauth_ctxt(conn,
                                                     (struct smb2_preauth_neg_context *)pctx,
-                                                    len_of_ctxts);
+                                                    ctxt_len);
                        if (status != STATUS_SUCCESS)
                                break;
                } else if (pctx->ContextType == SMB2_ENCRYPTION_CAPABILITIES) {
@@ -998,7 +1012,7 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
 
                        decode_encrypt_ctxt(conn,
                                            (struct smb2_encryption_neg_context *)pctx,
-                                           len_of_ctxts);
+                                           ctxt_len);
                } else if (pctx->ContextType == SMB2_COMPRESSION_CAPABILITIES) {
                        ksmbd_debug(SMB,
                                    "deassemble SMB2_COMPRESSION_CAPABILITIES context\n");
@@ -1017,9 +1031,10 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
                } else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) {
                        ksmbd_debug(SMB,
                                    "deassemble SMB2_SIGNING_CAPABILITIES context\n");
+
                        decode_sign_cap_ctxt(conn,
                                             (struct smb2_signing_capabilities *)pctx,
-                                            len_of_ctxts);
+                                            ctxt_len);
                }
 
                /* offsets must be 8 byte aligned */