[SCM] Samba Shared Repository - branch master updated

Douglas Bagnall dbagnall at samba.org
Sun Nov 17 23:55:02 UTC 2019


The branch, master has been updated
       via  e856877ef88 ndrdump: avoid use after free
       via  816869ecea0 ndrdump: Use human-readable strings for NDR decode errors
       via  b3bdb17a353 selftest: Test fix for ndrdump of structures by number
       via  2bb642d98e1 ndrdump: correctly find the public strict by number
      from  f1fa0d3b9df librpc: Use the fact that file_save() now uses O_EXCL in dcerpc_log_packet()

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit e856877ef88bf273cbf814ff17abad900ba7ea27
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Sat Nov 16 21:25:11 2019 +1300

    ndrdump: avoid use after free
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Douglas Bagnall <dbagnall at samba.org>
    Autobuild-Date(master): Sun Nov 17 23:54:11 UTC 2019 on sn-devel-184

commit 816869ecea06b0b936e3ead4074bb754ee8650ca
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Nov 15 19:25:54 2019 +1300

    ndrdump: Use human-readable strings for NDR decode errors
    
    These make much more sense than the NTSTATUS values they can be forced
    to map to.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>

commit b3bdb17a35380237f7b46cc2b453b6b6b7c7a4f8
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Nov 14 13:49:48 2019 +1300

    selftest: Test fix for ndrdump of structures by number
    
    This requires that misc.GUID not move in the IDL, so a comment is added.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14191
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>

commit 2bb642d98e1c26064907f8f671c1de864f2d8c2f
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 14 13:14:08 2019 +1300

    ndrdump: correctly find the public strict by number
    
    We were finding a function that happened to have the same ordinal
    number.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14191
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 librpc/idl/misc.idl                                |  6 ++
 librpc/tools/ndrdump.c                             | 68 ++++++++++------------
 python/samba/tests/blackbox/ndrdump.py             | 18 +++++-
 .../tests/dns-decode_dns_name_packet-hex.txt       |  2 +-
 source4/librpc/tests/misc-GUID.dat                 |  1 +
 5 files changed, 57 insertions(+), 38 deletions(-)
 create mode 100644 source4/librpc/tests/misc-GUID.dat


Changeset truncated at 500 lines:

diff --git a/librpc/idl/misc.idl b/librpc/idl/misc.idl
index 0b59ce5c96c..a705b53b5f3 100644
--- a/librpc/idl/misc.idl
+++ b/librpc/idl/misc.idl
@@ -11,6 +11,12 @@
 ]
 interface misc
 {
+	/*
+	 * While structures are not normally known by their order,
+	 * please keep this as the first struct, we use this for a
+	 * test of 'ndrdump misc 0 struct' (this helps debug failures
+	 * from our NDR fuzzing tool, which doesn't use string names)
+	 */
 	typedef [public,noprint,gensize] struct {
 		uint32 time_low;
 		uint16 time_mid;
diff --git a/librpc/tools/ndrdump.c b/librpc/tools/ndrdump.c
index 4173f03098d..e911cf4c1e4 100644
--- a/librpc/tools/ndrdump.c
+++ b/librpc/tools/ndrdump.c
@@ -66,6 +66,7 @@ static const struct ndr_interface_call *find_struct(
 	struct ndr_interface_call *out_buffer)
 {
 	unsigned int i;
+	const struct ndr_interface_public_struct *public_struct = NULL;
 	if (isdigit(struct_name[0])) {
 		char *eptr = NULL;
 		i = strtoul(struct_name, &eptr, 0);
@@ -76,23 +77,25 @@ static const struct ndr_interface_call *find_struct(
 			       struct_name);
 			exit(1);
 		}
-		return &p->calls[i];
-	}
-	for (i=0;i<p->num_public_structs;i++) {
-		if (strcmp(p->public_structs[i].name, struct_name) == 0) {
-			break;
+		public_struct = &p->public_structs[i];
+	} else {
+		for (i=0;i<p->num_public_structs;i++) {
+			if (strcmp(p->public_structs[i].name, struct_name) == 0) {
+				break;
+			}
 		}
-	}
-	if (i == p->num_public_structs) {
-		printf("Public structure '%s' not found\n", struct_name);
-		exit(1);
+		if (i == p->num_public_structs) {
+			printf("Public structure '%s' not found\n", struct_name);
+			exit(1);
+		}
+		public_struct = &p->public_structs[i];
 	}
 	*out_buffer = (struct ndr_interface_call) {
-		.name = p->public_structs[i].name,
-		.struct_size = p->public_structs[i].struct_size,
-		.ndr_pull = p->public_structs[i].ndr_pull,
-		.ndr_push = p->public_structs[i].ndr_push,
-		.ndr_print = p->public_structs[i].ndr_print
+		.name = public_struct->name,
+		.struct_size = public_struct->struct_size,
+		.ndr_pull = public_struct->ndr_pull,
+		.ndr_push = public_struct->ndr_push,
+		.ndr_print = public_struct->ndr_print
 	};
 	return out_buffer;
 }
@@ -190,7 +193,6 @@ static NTSTATUS ndrdump_pull_and_print_pipes(const char *function,
 				struct ndr_print *ndr_print,
 				const struct ndr_interface_call_pipes *pipes)
 {
-	NTSTATUS status;
 	enum ndr_err_code ndr_err;
 	uint32_t i;
 
@@ -218,18 +220,19 @@ static NTSTATUS ndrdump_pull_and_print_pipes(const char *function,
 			ndr_pull->current_mem_ctx = c;
 			ndr_err = pipes->pipes[i].ndr_pull(ndr_pull, NDR_SCALARS, c);
 			ndr_pull->current_mem_ctx = saved_mem_ctx;
-			status = ndr_map_error2ntstatus(ndr_err);
 
-			printf("pull returned %s\n", nt_errstr(status));
-			if (!NT_STATUS_IS_OK(status)) {
+			printf("pull returned %s\n",
+			       ndr_map_error2string(ndr_err));
+			if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 				talloc_free(c);
-				return status;
+				return ndr_map_error2ntstatus(ndr_err);
 			}
 			pipes->pipes[i].ndr_print(ndr_print, n, c);
-			talloc_free(c);
 			if (*count == 0) {
+				talloc_free(c);
 				break;
 			}
+			talloc_free(c);
 			idx++;
 		}
 	}
@@ -464,8 +467,8 @@ static void ndr_print_dummy(struct ndr_print *ndr, const char *format, ...)
 		}
 
 		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-			status = ndr_map_error2ntstatus(ndr_err);
-			printf("pull for context file returned %s\n", nt_errstr(status));
+			printf("pull for context file returned %s\n",
+			       ndr_map_error2string(ndr_err));
 			exit(1);
 		}
 		memcpy(v_st, st, f->struct_size);
@@ -510,10 +513,9 @@ static void ndr_print_dummy(struct ndr_print *ndr, const char *format, ...)
 	ndr_print->depth = 1;
 
 	ndr_err = ndr_pop_dcerpc_sec_verification_trailer(ndr_pull, mem_ctx, &sec_vt);
-	status = ndr_map_error2ntstatus(ndr_err);
-	if (!NT_STATUS_IS_OK(status)) {
+	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 		printf("ndr_pop_dcerpc_sec_verification_trailer returned %s\n",
-		       nt_errstr(status));
+		       ndr_map_error2string(ndr_err));
 	}
 
 	if (sec_vt != NULL && sec_vt->count.count > 0) {
@@ -540,11 +542,10 @@ static void ndr_print_dummy(struct ndr_print *ndr, const char *format, ...)
 	}
 
 	ndr_err = f->ndr_pull(ndr_pull, flags, st);
-	status = ndr_map_error2ntstatus(ndr_err);
+	printf("pull returned %s\n",
+	       ndr_map_error2string(ndr_err));
 
-	printf("pull returned %s\n", nt_errstr(status));
-
-	if (stop_on_parse_failure  && !NT_STATUS_IS_OK(status)) {
+	if (stop_on_parse_failure && !NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 		printf("not printing because --stop-on-parse-failure\n");
 		exit(1);
 	}
@@ -569,11 +570,6 @@ static void ndr_print_dummy(struct ndr_print *ndr, const char *format, ...)
 
 	f->ndr_print(ndr_print, format, flags, st);
 
-	if (!NT_STATUS_IS_OK(status)) {
-		printf("dump FAILED\n");
-		exit(1);
-	}
-
 	if (flags & NDR_IN) {
 		status = ndrdump_pull_and_print_pipes(format,
 						      ndr_pull,
@@ -628,8 +624,8 @@ static void ndr_print_dummy(struct ndr_print *ndr, const char *format, ...)
 		ndr_v_pull->flags |= LIBNDR_FLAG_REF_ALLOC;
 
 		ndr_err = f->ndr_pull(ndr_v_pull, flags, v_st);
-		status = ndr_map_error2ntstatus(ndr_err);
-		printf("pull returned %s\n", nt_errstr(status));
+		printf("pull returned %s\n",
+		       ndr_map_error2string(ndr_err));
 		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 			printf("validate pull FAILED\n");
 			exit(1);
diff --git a/python/samba/tests/blackbox/ndrdump.py b/python/samba/tests/blackbox/ndrdump.py
index b8f467f8e82..4e638c920d8 100644
--- a/python/samba/tests/blackbox/ndrdump.py
+++ b/python/samba/tests/blackbox/ndrdump.py
@@ -77,7 +77,7 @@ class NdrDumpTests(BlackboxTestCase):
     def test_ndrdump_with_binary_struct_name(self):
         # Prefix of the expected unparsed PAC data (without times, as
         # these vary by host)
-        expected = '''pull returned NT_STATUS_OK
+        expected = '''pull returned Success
     PAC_DATA: struct PAC_DATA
         num_buffers              : 0x00000005 (5)
         version                  : 0x00000000 (0)
@@ -94,3 +94,19 @@ class NdrDumpTests(BlackboxTestCase):
         self.assertEqual(actual[:len(expected)],
                          expected.encode('utf-8'))
         self.assertTrue(actual.endswith(b"dump OK\n"))
+
+    def test_ndrdump_with_binary_struct_number(self):
+        expected = '''pull returned Success
+    0                        : 33323130-3534-3736-3839-616263646566
+dump OK
+'''
+        try:
+            actual = self.check_output(
+                "ndrdump misc 0 struct %s" %
+                self.data_path("misc-GUID.dat"))
+        except BlackboxProcessError as e:
+            self.fail(e)
+
+        # check_output will return bytes
+        # convert expected to bytes for python 3
+        self.assertEqual(actual, expected.encode('utf-8'))
diff --git a/source4/librpc/tests/dns-decode_dns_name_packet-hex.txt b/source4/librpc/tests/dns-decode_dns_name_packet-hex.txt
index a973c28d5b9..02e95c0bd20 100644
--- a/source4/librpc/tests/dns-decode_dns_name_packet-hex.txt
+++ b/source4/librpc/tests/dns-decode_dns_name_packet-hex.txt
@@ -1,4 +1,4 @@
-pull returned NT_STATUS_OK
+pull returned Success
     dns_name_packet: struct dns_name_packet
         id                       : 0xecef (60655)
         operation                : 0x2800 (10240)
diff --git a/source4/librpc/tests/misc-GUID.dat b/source4/librpc/tests/misc-GUID.dat
new file mode 100644
index 00000000000..454f6b314bf
--- /dev/null
+++ b/source4/librpc/tests/misc-GUID.dat
@@ -0,0 +1 @@
+0123456789abcdef
\ No newline at end of file


-- 
Samba Shared Repository



More information about the samba-cvs mailing list