Fix inspired by work done by David Disseldorp for bug #8040 - smbclient segfaults when a Cyrillic netbios name or workgroup is configured.

Change msrpc_gen to return NTSTATUS and ensure everywhere this is
used it is correctly checked to return that status.

Jeremy.

5 files changed, 52 insertions, 19 deletions

diff --git a/libcli/auth/msrpc_parse.c b/libcli/auth/msrpc_parse.c
index 1351dfaae7..bdbba3d76c 100644
--- a/libcli/auth/msrpc_parse.c
+++ b/libcli/auth/msrpc_parse.c
@@ -40,7 +40,7 @@
   d = word (4 bytes)
   C = constant ascii string
  */
-bool msrpc_gen(TALLOC_CTX *mem_ctx, 
+NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, 
 	       DATA_BLOB *blob,
 	       const char *format, ...)
 {
@@ -57,7 +57,13 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
 	DATA_BLOB *pointers;
 
 	pointers = talloc_array(mem_ctx, DATA_BLOB, strlen(format));
+	if (!pointers) {
+		return NT_STATUS_NO_MEMORY;
+	}
 	intargs = talloc_array(pointers, int, strlen(format));
+	if (!intargs) {
+		return NT_STATUS_NO_MEMORY;
+	}
 
 	/* first scan the format to work out the header and body size */
 	va_start(ap, format);
@@ -72,7 +78,7 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
 				s, &n);
 			if (!ret) {
 				va_end(ap);
-				return false;
+				return map_nt_error_from_unix(errno);
 			}
 			pointers[i].length = n;
 			pointers[i].length -= 2;
@@ -86,7 +92,7 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
 				s, &n);
 			if (!ret) {
 				va_end(ap);
-				return false;
+				return map_nt_error_from_unix(errno);
 			}
 			pointers[i].length = n;
 			pointers[i].length -= 1;
@@ -102,7 +108,7 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
 				s, &n);
 			if (!ret) {
 				va_end(ap);
-				return false;
+				return map_nt_error_from_unix(errno);
 			}
 			pointers[i].length = n;
 			pointers[i].length -= 2;
@@ -132,13 +138,22 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
 			pointers[i].length = strlen(s)+1;
 			head_size += pointers[i].length;
 			break;
+		default:
+			va_end(ap);
+			return NT_STATUS_INVALID_PARAMETER;
 		}
 	}
 	va_end(ap);
 
+	if (head_size + data_size == 0) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
 	/* allocate the space, then scan the format again to fill in the values */
 	*blob = data_blob_talloc(mem_ctx, NULL, head_size + data_size);
-
+	if (!blob->data) {
+		return NT_STATUS_NO_MEMORY;
+	}
 	head_ofs = 0;
 	data_ofs = head_size;
 
@@ -185,13 +200,16 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
 			memcpy(blob->data + head_ofs, pointers[i].data, n);
 			head_ofs += n;
 			break;
+		default:
+			va_end(ap);
+			return NT_STATUS_INVALID_PARAMETER;
 		}
 	}
 	va_end(ap);
 	
 	talloc_free(pointers);
 
-	return true;
+	return NT_STATUS_OK;
 }
 
 
@@ -231,6 +249,10 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx,
 	char *p = talloc_array(mem_ctx, char, p_len);
 	bool ret = true;
 
+	if (!p) {
+		return false;
+	}
+
 	va_start(ap, format);
 	for (i=0; format[i]; i++) {
 		switch (format[i]) {
diff --git a/libcli/auth/msrpc_parse.h b/libcli/auth/msrpc_parse.h
index 507694db70..d976a95b73 100644
--- a/libcli/auth/msrpc_parse.h
+++ b/libcli/auth/msrpc_parse.h
@@ -11,7 +11,7 @@
 
 /* The following definitions come from /home/jeremy/src/samba/git/master/source3/../source4/../libcli/auth/msrpc_parse.c  */
 
-bool msrpc_gen(TALLOC_CTX *mem_ctx, 
+NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, 
 	       DATA_BLOB *blob,
 	       const char *format, ...);
 
diff --git a/libcli/auth/ntlmssp_server.c b/libcli/auth/ntlmssp_server.c
index 264e8bc908..802ac402b4 100644
--- a/libcli/auth/ntlmssp_server.c
+++ b/libcli/auth/ntlmssp_server.c
@@ -144,12 +144,15 @@ NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
 	/* This creates the 'blob' of names that appears at the end of the packet */
 	if (chal_flags & NTLMSSP_NEGOTIATE_TARGET_INFO)
 	{
-		msrpc_gen(ntlmssp_state, &struct_blob, "aaaaa",
+		status = msrpc_gen(ntlmssp_state, &struct_blob, "aaaaa",
 			  MsvAvNbDomainName, target_name,
 			  MsvAvNbComputerName, ntlmssp_state->server.netbios_name,
 			  MsvAvDnsDomainName, ntlmssp_state->server.dns_domain,
 			  MsvAvDnsComputerName, ntlmssp_state->server.dns_name,
 			  MsvAvEOL, "");
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
 	} else {
 		struct_blob = data_blob_null;
 	}
@@ -187,7 +190,7 @@ NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
 			gen_string = "CdAdbddBb";
 		}
 
-		msrpc_gen(out_mem_ctx, reply, gen_string,
+		status = msrpc_gen(out_mem_ctx, reply, gen_string,
 			"NTLMSSP",
 			NTLMSSP_CHALLENGE,
 			target_name,
@@ -197,6 +200,12 @@ NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
 			struct_blob.data, struct_blob.length,
 			version_blob.data, version_blob.length);
 
+		if (!NT_STATUS_IS_OK(status)) {
+			data_blob_free(&version_blob);
+			data_blob_free(&struct_blob);
+			return status;
+		}
+
 		data_blob_free(&version_blob);
 
 		if (DEBUGLEVEL >= 10) {
diff --git a/libcli/auth/ntlmssp_sign.c b/libcli/auth/ntlmssp_sign.c
index 0e57c07a8d..42b459c6d4 100644
--- a/libcli/auth/ntlmssp_sign.c
+++ b/libcli/auth/ntlmssp_sign.c
@@ -130,17 +130,17 @@ static NTSTATUS ntlmssp_make_packet_signature(struct ntlmssp_state *ntlmssp_stat
 		dump_data_pw("ntlmssp v2 sig ", sig->data, sig->length);
 
 	} else {
-		bool ok;
+		NTSTATUS status;
 		uint32_t crc;
 
 		crc = crc32_calc_buffer(data, length);
 
-		ok = msrpc_gen(sig_mem_ctx,
+		status = msrpc_gen(sig_mem_ctx,
 			       sig, "dddd",
 			       NTLMSSP_SIGN_VERSION, 0, crc,
 			       ntlmssp_state->crypt->ntlm.seq_num);
-		if (!ok) {
-			return NT_STATUS_NO_MEMORY;
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 
 		ntlmssp_state->crypt->ntlm.seq_num++;
@@ -307,17 +307,17 @@ NTSTATUS ntlmssp_seal_packet(struct ntlmssp_state *ntlmssp_state,
 					   sig->data+4, 8);
 		}
 	} else {
-		bool ok;
+		NTSTATUS status;
 		uint32_t crc;
 
 		crc = crc32_calc_buffer(data, length);
 
-		ok = msrpc_gen(sig_mem_ctx,
+		status = msrpc_gen(sig_mem_ctx,
 			       sig, "dddd",
 			       NTLMSSP_SIGN_VERSION, 0, crc,
 			       ntlmssp_state->crypt->ntlm.seq_num);
-		if (!ok) {
-			return NT_STATUS_NO_MEMORY;
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 
 		/*
diff --git a/libcli/auth/smbencrypt.c b/libcli/auth/smbencrypt.c
index abd8ad92a3..825739ac4b 100644
--- a/libcli/auth/smbencrypt.c
+++ b/libcli/auth/smbencrypt.c
@@ -363,7 +363,8 @@ DATA_BLOB NTLMv2_generate_names_blob(TALLOC_CTX *mem_ctx,
 {
 	DATA_BLOB names_blob = data_blob_talloc(mem_ctx, NULL, 0);
 
-	msrpc_gen(mem_ctx, &names_blob,
+	/* Deliberately ignore return here.. */
+	(void)msrpc_gen(mem_ctx, &names_blob,
 		  "aaa",
 		  MsvAvNbDomainName, domain,
 		  MsvAvNbComputerName, hostname,
@@ -386,7 +387,8 @@ static DATA_BLOB NTLMv2_generate_client_data(TALLOC_CTX *mem_ctx, const DATA_BLO
 
 	/* See http://www.ubiqx.org/cifs/SMB.html#SMB.8.5 */
 
-	msrpc_gen(mem_ctx, &response, "ddbbdb",
+	/* Deliberately ignore return here.. */
+	(void)msrpc_gen(mem_ctx, &response, "ddbbdb",
 		  0x00000101,     /* Header  */
 		  0,              /* 'Reserved'  */
 		  long_date, 8,	  /* Timestamp */