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)

2 files changed, 46 insertions, 14 deletions

diff --git a/source3/libsmb/clispnego.c b/source3/libsmb/clispnego.c
index eabddd1765..54069fc637 100644
--- 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 969508a360..48df7fc564 100644
--- 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;
 	}
 }