NTLMSSP parinoia - we really don't want to run over the end of our blob,

and make sure we can never get an 'authenticate' packet without a challenge.

Andrew Bartlett
(This used to be commit 4d94f8e6912c1339515cd1f68d1b698e7c699626)

diff --git a/source3/libsmb/clispnego.c b/source3/libsmb/clispnego.c
index eabddd1765f18e57dcd90f46df3ac1bc888a8fd0..54069fc63745089ba4e00147829b29a0b6547fa1 100644 (file)
--- a/source3/libsmb/clispnego.c
+++ b/source3/libsmb/clispnego.c
@@ -636,6 +636,12 @@ BOOL msrpc_gen(DATA_BLOB *blob,
 }
 
 
+/* a helpful macro to avoid running over the end of our blob */
+#define NEED_DATA(amount) \
+if (head_ofs + amount > blob->length) { \
+        return False; \
+}
+
 /*
   this is a tiny msrpc packet parser. This the the partner of msrpc_gen
 
@@ -648,6 +654,7 @@ BOOL msrpc_gen(DATA_BLOB *blob,
   d = word (4 bytes)
   C = constant ascii string
  */
+
 BOOL msrpc_parse(DATA_BLOB *blob,
                 const char *format, ...)
 {
@@ -665,19 +672,32 @@ BOOL msrpc_parse(DATA_BLOB *blob,
        for (i=0; format[i]; i++) {
                switch (format[i]) {
                case 'U':
+                       NEED_DATA(8);
                        len1 = SVAL(blob->data, head_ofs); head_ofs += 2;
                        len2 = SVAL(blob->data, head_ofs); head_ofs += 2;
                        ptr =  IVAL(blob->data, head_ofs); head_ofs += 4;
+
                        /* make sure its in the right format - be strict */
-                       if (len1 != len2 || (len1&1) || ptr + len1 > blob->length) {
+                       if (len1 != len2 || ptr + len1 > blob->length) {
+                               return False;
+                       }
+                       if (len1 & 1) {
+                               /* if odd length and unicode */
                                return False;
                        }
+
                        ps = va_arg(ap, char **);
-                       pull_string(NULL, p, blob->data + ptr, -1, len1, 
-                                   STR_UNICODE|STR_NOALIGN);
-                       (*ps) = strdup(p);
+                       if (0 < len1) {
+                               pull_string(NULL, p, blob->data + ptr, sizeof(p), 
+                                           len1, 
+                                           STR_UNICODE|STR_NOALIGN);
+                               (*ps) = strdup(p);
+                       } else {
+                               (*ps) = NULL;
+                       }
                        break;
                case 'A':
+                       NEED_DATA(8);
                        len1 = SVAL(blob->data, head_ofs); head_ofs += 2;
                        len2 = SVAL(blob->data, head_ofs); head_ofs += 2;
                        ptr =  IVAL(blob->data, head_ofs); head_ofs += 4;
@@ -686,16 +706,19 @@ BOOL msrpc_parse(DATA_BLOB *blob,
                        if (len1 != len2 || ptr + len1 > blob->length) {
                                return False;
                        }
+
                        ps = va_arg(ap, char **);
                        if (0 < len1) {
-                               pull_string(NULL, p, blob->data + ptr, -1, 
-                                           len1, STR_ASCII|STR_NOALIGN);
+                               pull_string(NULL, p, blob->data + ptr, sizeof(p), 
+                                           len1, 
+                                           STR_ASCII|STR_NOALIGN);
                                (*ps) = strdup(p);
                        } else {
                                (*ps) = NULL;
                        }
                        break;
                case 'B':
+                       NEED_DATA(8);
                        len1 = SVAL(blob->data, head_ofs); head_ofs += 2;
                        len2 = SVAL(blob->data, head_ofs); head_ofs += 2;
                        ptr =  IVAL(blob->data, head_ofs); head_ofs += 4;
@@ -709,12 +732,14 @@ BOOL msrpc_parse(DATA_BLOB *blob,
                case 'b':
                        b = (DATA_BLOB *)va_arg(ap, void *);
                        len1 = va_arg(ap, unsigned);
-                       *b = data_blob(blob->data + head_ofs, 
-                                      MIN(len1, blob->length - head_ofs));
+                       /* make sure its in the right format - be strict */
+                       NEED_DATA(len1);
+                       *b = data_blob(blob->data + head_ofs, len1);
                        head_ofs += len1;
                        break;
                case 'd':
                        v = va_arg(ap, uint32 *);
+                       NEED_DATA(4);
                        *v = IVAL(blob->data, head_ofs); head_ofs += 4;
                        break;
                case 'C':

diff --git a/source3/libsmb/ntlmssp.c b/source3/libsmb/ntlmssp.c
index 969508a360af2461022e09c1c75b481457eb8505..48df7fc564d19a5973b0962a630ddb2c31f36ad1 100644 (file)
--- a/source3/libsmb/ntlmssp.c
+++ b/source3/libsmb/ntlmssp.c
@@ -76,7 +76,7 @@ static NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
                         &neg_flags,
                         &cliname,
                         &domname)) {
-               return NT_STATUS_LOGON_FAILURE;
+               return NT_STATUS_INVALID_PARAMETER;
        }
 
        SAFE_FREE(cliname);
@@ -155,6 +155,8 @@ static NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
                
        data_blob_free(&struct_blob);
 
+       ntlmssp_state->expected_state = NTLMSSP_AUTH;
+
        return NT_STATUS_MORE_PROCESSING_REQUIRED;
 }
 
@@ -196,7 +198,7 @@ static NTSTATUS ntlmssp_server_auth(struct ntlmssp_state *ntlmssp_state,
                         &ntlmssp_state->workstation,
                         &sess_key,
                         &neg_flags)) {
-               return NT_STATUS_LOGON_FAILURE;
+               return NT_STATUS_INVALID_PARAMETER;
        }
 
        data_blob_free(&sess_key);
@@ -224,7 +226,7 @@ NTSTATUS ntlmssp_server_start(NTLMSSP_STATE **ntlmssp_state)
        
        *ntlmssp_state = talloc_zero(mem_ctx, sizeof(**ntlmssp_state));
        if (!*ntlmssp_state) {
-               DEBUG(0,("ntlmssp_start: talloc failed!\n"));
+               DEBUG(0,("ntlmssp_server_start: talloc failed!\n"));
                talloc_destroy(mem_ctx);
                return NT_STATUS_NO_MEMORY;
        }
@@ -238,6 +240,8 @@ NTSTATUS ntlmssp_server_start(NTLMSSP_STATE **ntlmssp_state)
        (*ntlmssp_state)->get_domain = lp_workgroup;
        (*ntlmssp_state)->server_role = ROLE_DOMAIN_MEMBER; /* a good default */
 
+       (*ntlmssp_state)->expected_state = NTLMSSP_NEGOTIATE;
+
        return NT_STATUS_OK;
 }
 
@@ -267,8 +271,11 @@ NTSTATUS ntlmssp_server_update(NTLMSSP_STATE *ntlmssp_state,
        if (!msrpc_parse(&request, "Cd",
                         "NTLMSSP",
                         &ntlmssp_command)) {
-               
-               return NT_STATUS_LOGON_FAILURE;
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+
+       if (ntlmssp_command != ntlmssp_state->expected_state) {
+               return NT_STATUS_INVALID_PARAMETER;
        }
 
        if (ntlmssp_command == NTLMSSP_NEGOTIATE) {
@@ -276,7 +283,7 @@ NTSTATUS ntlmssp_server_update(NTLMSSP_STATE *ntlmssp_state,
        } else if (ntlmssp_command == NTLMSSP_AUTH) {
                return ntlmssp_server_auth(ntlmssp_state, request, reply);
        } else {
-               return NT_STATUS_LOGON_FAILURE;
+               return NT_STATUS_INVALID_PARAMETER;
        }
 }