Commit 5105a7ff authored by David Disseldorp's avatar David Disseldorp Committed by Steve French

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: 5100d8a3 ("SMB311: Improve checking of negotiate security contexts")
Reported-by: default avatarVolker Lendecke <vl@samba.org>
Reviewed-by: default avatarPaulo Alcantara (SUSE) <pc@manguebit.com>
Signed-off-by: default avatarDavid Disseldorp <ddiss@suse.de>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 09a9639e
...@@ -587,11 +587,15 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, ...@@ -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) static void decode_preauth_context(struct smb2_preauth_neg_context *ctxt)
{ {
unsigned int len = le16_to_cpu(ctxt->DataLength); 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) { if (len < MIN_PREAUTH_CTXT_DATA_LEN) {
pr_warn_once("server sent bad preauth context\n"); pr_warn_once("server sent bad preauth context\n");
return; return;
...@@ -610,7 +614,11 @@ static void decode_compress_ctx(struct TCP_Server_Info *server, ...@@ -610,7 +614,11 @@ static void decode_compress_ctx(struct TCP_Server_Info *server,
{ {
unsigned int len = le16_to_cpu(ctxt->DataLength); 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) { if (len < 10) {
pr_warn_once("server sent bad compression cntxt\n"); pr_warn_once("server sent bad compression cntxt\n");
return; return;
...@@ -632,6 +640,11 @@ static int decode_encrypt_ctx(struct TCP_Server_Info *server, ...@@ -632,6 +640,11 @@ static int decode_encrypt_ctx(struct TCP_Server_Info *server,
unsigned int len = le16_to_cpu(ctxt->DataLength); unsigned int len = le16_to_cpu(ctxt->DataLength);
cifs_dbg(FYI, "decode SMB3.11 encryption neg context of len %d\n", len); 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) { if (len < MIN_ENCRYPT_CTXT_DATA_LEN) {
pr_warn_once("server sent bad crypto ctxt len\n"); pr_warn_once("server sent bad crypto ctxt len\n");
return -EINVAL; return -EINVAL;
...@@ -678,6 +691,11 @@ static void decode_signing_ctx(struct TCP_Server_Info *server, ...@@ -678,6 +691,11 @@ static void decode_signing_ctx(struct TCP_Server_Info *server,
{ {
unsigned int len = le16_to_cpu(pctxt->DataLength); 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)) { if ((len < 4) || (len > 16)) {
pr_warn_once("server sent bad signing negcontext\n"); pr_warn_once("server sent bad signing negcontext\n");
return; return;
...@@ -719,14 +737,19 @@ static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp, ...@@ -719,14 +737,19 @@ static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp,
for (i = 0; i < ctxt_cnt; i++) { for (i = 0; i < ctxt_cnt; i++) {
int clen; int clen;
/* check that offset is not beyond end of SMB */ /* check that offset is not beyond end of SMB */
if (len_of_ctxts == 0)
break;
if (len_of_ctxts < sizeof(struct smb2_neg_context)) if (len_of_ctxts < sizeof(struct smb2_neg_context))
break; break;
pctx = (struct smb2_neg_context *)(offset + (char *)rsp); 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) if (clen > len_of_ctxts)
break; break;
...@@ -747,12 +770,10 @@ static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp, ...@@ -747,12 +770,10 @@ static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp,
else else
cifs_server_dbg(VFS, "unknown negcontext of type %d ignored\n", cifs_server_dbg(VFS, "unknown negcontext of type %d ignored\n",
le16_to_cpu(pctx->ContextType)); le16_to_cpu(pctx->ContextType));
if (rc) if (rc)
break; break;
/* offsets must be 8 byte aligned */
clen = ALIGN(clen, 8); offset += clen;
offset += clen + sizeof(struct smb2_neg_context);
len_of_ctxts -= clen; len_of_ctxts -= clen;
} }
return rc; return rc;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment