[PATCHES] Fix sd and devmode ptr handling in various spoolss calls

Günther Deschner gd at samba.org
Fri Nov 11 18:37:52 UTC 2016


Hi,

attached are some patches that fix our incorrect handling of security
descriptor and device mode pointers for spoolss set calls (the IDL
generated, not the handmarshalled ones).

More details about ULONG_PTR can be found in the MIDL 64bit porting
guide: https://msdn.microsoft.com/en-us/library/ms810720.aspx

Please review and push,

Thanks,
Guenther
-- 
Günther Deschner                    GPG-ID: 8EE11688
Red Hat                         gdeschner at redhat.com
Samba Team                              gd at samba.org
-------------- next part --------------
From e05afeab7a97a71b7ab3ddcef421cd76a9ba84ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd at samba.org>
Date: Fri, 11 Nov 2016 16:29:20 +0100
Subject: [PATCH 1/2] spoolss: Use correct values for secdesc and devmode
 pointers

ULONG_PTR needs to be decoded as a uint3264 and not as a 'uint32 *'.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11197

Guenther

Pair-Programmed-With: Andreas Schneider <asn at samba.org>

Signed-off-by: Guenther Deschner <gd at samba.org>
Signed-off-by: Andreas Schneider <asn at samba.org>
---
 librpc/idl/spoolss.idl               | 20 ++++++++++----------
 source3/rpc_client/init_spoolss.c    |  4 ++--
 source3/rpcclient/cmd_spoolss.c      |  4 ++--
 source4/torture/rpc/spoolss.c        | 14 +++++++-------
 source4/torture/rpc/spoolss_notify.c |  4 ++--
 5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/librpc/idl/spoolss.idl b/librpc/idl/spoolss.idl
index df19bee..6ab8f03 100644
--- a/librpc/idl/spoolss.idl
+++ b/librpc/idl/spoolss.idl
@@ -1011,9 +1011,9 @@ cpp_quote("#define spoolss_security_descriptor security_descriptor")
 		[string,charset(UTF16)] uint16 *print_processor;
 		[string,charset(UTF16)] uint16 *parameters;
 		[string,charset(UTF16)] uint16 *driver_name;
-		uint32 *_devmode_ptr; /* pointer to truncated devicemode */
+		uint3264 _devmode_ptr; /* ULONG_PTR */
 		[string,charset(UTF16)] uint16 *text_status;
-		uint32 *_secdesc_ptr;
+		uint3264 _secdesc_ptr; /* ULONG_PTR */
 		spoolss_JobStatus status;
 		[range(0,99)] uint32 priority;
 		uint32 position;
@@ -1037,9 +1037,9 @@ cpp_quote("#define spoolss_security_descriptor security_descriptor")
 		[string,charset(UTF16)] uint16 *print_processor;
 		[string,charset(UTF16)] uint16 *parameters;
 		[string,charset(UTF16)] uint16 *driver_name;
-		uint32 *_devmode_ptr; /* pointer to truncated devicemode */
+		uint3264 _devmode_ptr; /* ULONG_PTR */
 		[string,charset(UTF16)] uint16 *text_status;
-		uint32 *_secdesc_ptr;
+		uint3264 _secdesc_ptr; /* ULONG_PTR */
 		spoolss_JobStatus status;
 		[range(0,99)] uint32 priority;
 		uint32 position;
@@ -1201,12 +1201,12 @@ cpp_quote("#define spoolss_security_descriptor security_descriptor")
 		[string,charset(UTF16)] uint16 *drivername;
 		[string,charset(UTF16)] uint16 *comment;
 		[string,charset(UTF16)] uint16 *location;
-		uint32 *devmode_ptr;
+		uint3264 devmode_ptr; /* ULONG_PTR */
 		[string,charset(UTF16)] uint16 *sepfile;
 		[string,charset(UTF16)] uint16 *printprocessor;
 		[string,charset(UTF16)] uint16 *datatype;
 		[string,charset(UTF16)] uint16 *parameters;
-		uint32 *secdesc_ptr;
+		uint3264 secdesc_ptr; /* ULONG_PTR */
 		spoolss_PrinterAttributes attributes;
 		[range(0,99)] uint32 priority;
 		uint32 defaultpriority;
@@ -1218,7 +1218,7 @@ cpp_quote("#define spoolss_security_descriptor security_descriptor")
 	} spoolss_SetPrinterInfo2;
 
 	typedef struct {
-		uint32 *sec_desc_ptr;
+		uint3264 sec_desc_ptr; /* ULONG_PTR */
 	} spoolss_SetPrinterInfo3;
 
 	typedef struct {
@@ -1245,11 +1245,11 @@ cpp_quote("#define spoolss_security_descriptor security_descriptor")
 	} spoolss_SetPrinterInfo7;
 
 	typedef struct {
-		uint32 *devmode_ptr;
+		uint3264 devmode_ptr; /* ULONG_PTR */
 	} spoolss_SetPrinterInfo8;
 
 	typedef struct {
-		uint32 *devmode_ptr;
+		uint3264 devmode_ptr; /* ULONG_PTR */
 	} spoolss_SetPrinterInfo9;
 
 	typedef [ms_union,switch_type(uint32)] union {
@@ -2307,7 +2307,7 @@ cpp_quote("#define spoolss_security_descriptor security_descriptor")
 	/* Function: 0x27 */
 	WERROR spoolss_DeletePort(
 	       [in,unique] [string,charset(UTF16)] uint16 *server_name,
-	       [in] uint32 ptr,
+	       [in] uint3264 ptr, /* ULONG_PTR */
 	       [in,ref] [string,charset(UTF16)] uint16 *port_name
 	);
 
diff --git a/source3/rpc_client/init_spoolss.c b/source3/rpc_client/init_spoolss.c
index 512cc56..d80d453 100644
--- a/source3/rpc_client/init_spoolss.c
+++ b/source3/rpc_client/init_spoolss.c
@@ -157,12 +157,12 @@ void spoolss_printerinfo2_to_setprinterinfo2(const struct spoolss_PrinterInfo2 *
 	s->drivername		= i->drivername;
 	s->comment		= i->comment;
 	s->location		= i->location;
-	s->devmode_ptr		= NULL;
+	s->devmode_ptr		= 0;
 	s->sepfile		= i->sepfile;
 	s->printprocessor	= i->printprocessor;
 	s->datatype		= i->datatype;
 	s->parameters		= i->parameters;
-	s->secdesc_ptr		= NULL;
+	s->secdesc_ptr		= 0;
 	s->attributes		= i->attributes;
 	s->priority		= i->priority;
 	s->defaultpriority	= i->defaultpriority;
diff --git a/source3/rpcclient/cmd_spoolss.c b/source3/rpcclient/cmd_spoolss.c
index 55d41c8..00dc2e0 100644
--- a/source3/rpcclient/cmd_spoolss.c
+++ b/source3/rpcclient/cmd_spoolss.c
@@ -1893,8 +1893,8 @@ static WERROR cmd_spoolss_addprinterex(struct rpc_pipe_client *cli,
 	info2.comment		= "Created by rpcclient";
 	info2.printprocessor	= "winprint";
 	info2.datatype		= "RAW";
-	info2.devmode_ptr	= NULL;
-	info2.secdesc_ptr	= NULL;
+	info2.devmode_ptr	= 0;
+	info2.secdesc_ptr	= 0;
 	info2.attributes 	= PRINTER_ATTRIBUTE_SHARED;
 	info2.priority 		= 0;
 	info2.defaultpriority	= 0;
diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
index dc33917..28abd14 100644
--- a/source4/torture/rpc/spoolss.c
+++ b/source4/torture/rpc/spoolss.c
@@ -244,12 +244,12 @@ static bool PrinterInfo_to_SetPrinterInfo(struct torture_context *tctx,
 		s->info2->drivername		= i->info2.drivername;
 		s->info2->comment		= i->info2.comment;
 		s->info2->location		= i->info2.location;
-		s->info2->devmode_ptr		= NULL;
+		s->info2->devmode_ptr		= 0;
 		s->info2->sepfile		= i->info2.sepfile;
 		s->info2->printprocessor	= i->info2.printprocessor;
 		s->info2->datatype		= i->info2.datatype;
 		s->info2->parameters		= i->info2.parameters;
-		s->info2->secdesc_ptr		= NULL;
+		s->info2->secdesc_ptr		= 0;
 		s->info2->attributes		= i->info2.attributes;
 		s->info2->priority		= i->info2.priority;
 		s->info2->defaultpriority	= i->info2.defaultpriority;
@@ -1459,8 +1459,8 @@ static bool test_SetPrinter_errors(struct torture_context *tctx,
 static void clear_info2(struct spoolss_SetPrinterInfoCtr *r)
 {
 	if ((r->level == 2) && (r->info.info2)) {
-		r->info.info2->secdesc_ptr = NULL;
-		r->info.info2->devmode_ptr = NULL;
+		r->info.info2->secdesc_ptr = 0;
+		r->info.info2->devmode_ptr = 0;
 	}
 }
 
@@ -1933,7 +1933,7 @@ static bool test_sd_set_level(struct torture_context *tctx,
 	}
 	case 3: {
 
-		info3.sec_desc_ptr = NULL;
+		info3.sec_desc_ptr = 0;
 
 		info_ctr.level = 3;
 		info_ctr.info.info3 = &info3;
@@ -2104,7 +2104,7 @@ static bool test_devmode_set_level(struct torture_context *tctx,
 	case 8: {
 		struct spoolss_SetPrinterInfo8 info8;
 
-		info8.devmode_ptr = NULL;
+		info8.devmode_ptr = 0;
 
 		info_ctr.level = 8;
 		info_ctr.info.info8 = &info8;
@@ -8231,7 +8231,7 @@ static bool test_set_printer_printserverhandle(struct torture_context *tctx,
 
 	secdesc_ctr.sd = sd;
 
-	info3.sec_desc_ptr = NULL;
+	info3.sec_desc_ptr = 0;
 
 	info_ctr.level = 3;
 	info_ctr.info.info3 = &info3;
diff --git a/source4/torture/rpc/spoolss_notify.c b/source4/torture/rpc/spoolss_notify.c
index 945e653..928b619 100644
--- a/source4/torture/rpc/spoolss_notify.c
+++ b/source4/torture/rpc/spoolss_notify.c
@@ -391,12 +391,12 @@ static bool test_SetPrinter(struct torture_context *tctx,
 	info2.drivername	= info.info2.drivername;
 	info2.comment		= talloc_asprintf(tctx, "torture_comment %d\n", (int)time(NULL));
 	info2.location		= info.info2.location;
-	info2.devmode_ptr	= NULL;
+	info2.devmode_ptr	= 0;
 	info2.sepfile		= info.info2.sepfile;
 	info2.printprocessor	= info.info2.printprocessor;
 	info2.datatype		= info.info2.datatype;
 	info2.parameters	= info.info2.parameters;
-	info2.secdesc_ptr	= NULL;
+	info2.secdesc_ptr	= 0;
 	info2.attributes	= info.info2.attributes;
 	info2.priority		= info.info2.priority;
 	info2.defaultpriority	= info.info2.defaultpriority;
-- 
2.7.4


From 9cbd7574b3b13702cd12a709194940a84ea66f91 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd at samba.org>
Date: Fri, 11 Nov 2016 19:17:55 +0100
Subject: [PATCH 2/2] s4-torture: add spoolss_SetPrinter ndr test to validate
 secdesc_ptr

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11197

Guenther

Signed-off-by: Guenther Deschner <gd at samba.org>
---
 source4/torture/ndr/spoolss.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/source4/torture/ndr/spoolss.c b/source4/torture/ndr/spoolss.c
index c8c466d..72a4f5b 100644
--- a/source4/torture/ndr/spoolss.c
+++ b/source4/torture/ndr/spoolss.c
@@ -1769,6 +1769,61 @@ static bool setjobnamedproperty_req_check(struct torture_context *tctx,
 	return true;
 }
 
+static const uint8_t setprinter_level_3_xpsp3_req_data[] = {
+	0x00, 0x00, 0x00, 0x00, 0x3c, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x2b, 0x55, 0x94, 0xbe, 0x50, 0x28, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00,
+	0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x08, 0xd1, 0xe9, 0x06,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xb4, 0x00, 0x00, 0x00,
+	0x04, 0x00, 0x02, 0x00, 0xb4, 0x00, 0x00, 0x00, 0x01, 0x00, 0x04, 0x80,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x14, 0x00, 0x00, 0x00, 0x02, 0x00, 0xa0, 0x00, 0x06, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x18, 0x00, 0x0c, 0x00, 0x0f, 0x00, 0x01, 0x02, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x05, 0x20, 0x00, 0x00, 0x00, 0x20, 0x02, 0x00, 0x00,
+	0x00, 0x09, 0x18, 0x00, 0x30, 0x00, 0x0f, 0x00, 0x01, 0x02, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x05, 0x20, 0x00, 0x00, 0x00, 0x20, 0x02, 0x00, 0x00,
+	0x00, 0x00, 0x24, 0x00, 0x08, 0x00, 0x02, 0x00, 0x01, 0x05, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x05, 0x15, 0x00, 0x00, 0x00, 0xa4, 0xc0, 0x7d, 0x3b,
+	0xcc, 0xce, 0x29, 0xa7, 0xd1, 0xc7, 0xe9, 0xd4, 0x50, 0x04, 0x00, 0x00,
+	0x00, 0x00, 0x18, 0x00, 0x0c, 0x00, 0x0f, 0x00, 0x01, 0x02, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x05, 0x20, 0x00, 0x00, 0x00, 0x26, 0x02, 0x00, 0x00,
+	0x00, 0x09, 0x18, 0x00, 0x30, 0x00, 0x0f, 0x00, 0x01, 0x02, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x05, 0x20, 0x00, 0x00, 0x00, 0x26, 0x02, 0x00, 0x00,
+	0x00, 0x00, 0x14, 0x00, 0x08, 0x00, 0x02, 0x00, 0x01, 0x01, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+static bool setprinter_level_3_xpsp3_req_check(struct torture_context *tctx,
+					       struct spoolss_SetPrinter *r)
+{
+	struct GUID guid;
+
+	torture_assert_ntstatus_ok(tctx,
+		GUID_from_string("0000053c-0000-0000-2b55-94be50280000", &guid),
+		"failed to parse GUID");
+	torture_assert_int_equal(tctx, r->in.handle->handle_type, 0, "handle_type");
+	torture_assert_guid_equal(tctx, r->in.handle->uuid, guid, "handle.uuid");
+
+	torture_assert(tctx, r->in.info_ctr, "info_ctr");
+	torture_assert_int_equal(tctx, r->in.info_ctr->level, 3, "level");
+	torture_assert_int_equal(tctx, r->in.info_ctr->info.info3->sec_desc_ptr, 0x06e9d108, "sec_desc_ptr");
+
+	torture_assert(tctx, r->in.devmode_ctr, "devmode_ctr");
+	torture_assert_int_equal(tctx, r->in.devmode_ctr->_ndr_size, 0, "_ndr_size");
+	torture_assert(tctx, r->in.devmode_ctr->devmode == NULL, "devmode");
+
+	torture_assert(tctx, r->in.secdesc_ctr, "secdesc_ctr");
+	torture_assert_int_equal(tctx, r->in.secdesc_ctr->sd_size, 0x000000b4, "sd_size");
+	torture_assert_int_equal(tctx, r->in.secdesc_ctr->sd->revision, SECURITY_DESCRIPTOR_REVISION_1, "revision");
+	torture_assert_int_equal(tctx, r->in.secdesc_ctr->sd->type, 0x8004, "type");
+	torture_assert(tctx, r->in.secdesc_ctr->sd, "sd");
+	torture_assert(tctx, r->in.secdesc_ctr->sd->owner_sid == NULL, "owner_sid");
+	torture_assert(tctx, r->in.secdesc_ctr->sd->group_sid == NULL, "group_sid");
+	torture_assert(tctx, r->in.secdesc_ctr->sd->sacl == NULL, "sacl");
+	torture_assert(tctx, r->in.secdesc_ctr->sd->dacl, "dacl");
+
+	return true;
+}
+
 struct torture_suite *ndr_spoolss_suite(TALLOC_CTX *ctx)
 {
 	struct torture_suite *suite = torture_suite_create(ctx, "spoolss");
@@ -1859,5 +1914,8 @@ struct torture_suite *ndr_spoolss_suite(TALLOC_CTX *ctx)
 	torture_suite_add_ndr_pull_fn_test(suite, spoolss_SetJobNamedProperty, setjobnamedproperty_req_data, NDR_IN, setjobnamedproperty_req_check);
 	torture_suite_add_ndr_pull_fn_test(suite, winspool_AsyncSetJobNamedProperty, setjobnamedproperty_req_data, NDR_IN, NULL);
 
+	torture_suite_add_ndr_pull_fn_test(suite, spoolss_SetPrinter, setprinter_level_3_xpsp3_req_data, NDR_IN, setprinter_level_3_xpsp3_req_check);
+	torture_suite_add_ndr_pull_fn_test(suite, winspool_AsyncSetPrinter, setprinter_level_3_xpsp3_req_data, NDR_IN, NULL);
+
 	return suite;
 }
-- 
2.7.4

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 201 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161111/06e57519/signature.sig>


More information about the samba-technical mailing list