From: Andrew Bartlett <abartlet@samba.org>
Date: Sat, 20 Dec 2008 01:05:48 +0000 (+1100)
Subject: Don't trust sscanf not to run off the end of the string
X-Git-Url: http://git.samba.org/samba.git/?p=kai%2Fsamba.git;a=commitdiff_plain;h=18c095e5d86d1353eff8aea1b641968d504b6c80

Don't trust sscanf not to run off the end of the string

The memory allocations here are wasteful, but they do nicely ensure we
cannot walk off the end of the DATA_BLOB that might be a string, or
might be binary and might not be NULL terminated.

Andrew Bartlett
---

diff --git a/librpc/ndr/uuid.c b/librpc/ndr/uuid.c
index aa24ac44945..2b472468061 100644
--- a/librpc/ndr/uuid.c
+++ b/librpc/ndr/uuid.c
@@ -36,6 +36,7 @@ _PUBLIC_ NTSTATUS GUID_from_data_blob(const DATA_BLOB *s, struct GUID *guid)
 	uint32_t clock_seq[2];
 	uint32_t node[6];
 	uint8_t buf16[16];
+
 	DATA_BLOB blob16 = data_blob_const(buf16, sizeof(buf16));
 	int i;
 
@@ -43,20 +44,40 @@ _PUBLIC_ NTSTATUS GUID_from_data_blob(const DATA_BLOB *s, struct GUID *guid)
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	if (s->length == 36 && 
-	    11 == sscanf((const char *)s->data, 
-			 "%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
-			 &time_low, &time_mid, &time_hi_and_version, 
-			 &clock_seq[0], &clock_seq[1],
-			 &node[0], &node[1], &node[2], &node[3], &node[4], &node[5])) {
-	        status = NT_STATUS_OK;
-	} else if (s->length == 38
-		   && 11 == sscanf((const char *)s->data, 
-				   "{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}",
-				   &time_low, &time_mid, &time_hi_and_version, 
-				   &clock_seq[0], &clock_seq[1],
-				   &node[0], &node[1], &node[2], &node[3], &node[4], &node[5])) {
-		status = NT_STATUS_OK;
+	if (s->length == 36) {
+		TALLOC_CTX *mem_ctx;
+		const char *string;
+
+		mem_ctx = talloc_new(NULL);
+		NT_STATUS_HAVE_NO_MEMORY(mem_ctx);
+		string = talloc_strndup(mem_ctx, (const char *)s->data, s->length);
+		NT_STATUS_HAVE_NO_MEMORY(string);
+		if (11 == sscanf(string,
+				 "%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+				 &time_low, &time_mid, &time_hi_and_version, 
+				 &clock_seq[0], &clock_seq[1],
+				 &node[0], &node[1], &node[2], &node[3], &node[4], &node[5])) {
+			status = NT_STATUS_OK;
+		}
+		talloc_free(mem_ctx);
+
+	} else if (s->length == 38) {
+		TALLOC_CTX *mem_ctx;
+		const char *string;
+
+		mem_ctx = talloc_new(NULL);
+		NT_STATUS_HAVE_NO_MEMORY(mem_ctx);
+		string = talloc_strndup(mem_ctx, (const char *)s->data, s->length);
+		NT_STATUS_HAVE_NO_MEMORY(string);
+		if (11 == sscanf((const char *)s->data, 
+				 "{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}",
+				 &time_low, &time_mid, &time_hi_and_version, 
+				 &clock_seq[0], &clock_seq[1],
+				 &node[0], &node[1], &node[2], &node[3], &node[4], &node[5])) {
+			status = NT_STATUS_OK;
+		}
+		talloc_free(mem_ctx);
+
 	} else if (s->length == 32) {
 		size_t rlen = strhex_to_str((char *)blob16.data, blob16.length,
 					    (const char *)s->data, s->length);