[PATCHES] Fix sd and devmode ptr handling in various spoolss calls
Jeremy Allison
jra at samba.org
Fri Nov 11 21:25:02 UTC 2016
On Fri, Nov 11, 2016 at 07:37:52PM +0100, Günther Deschner wrote:
> 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,
Great work you two ! Thanks Günther for the regression
test.
Pushed.
Jeremy.
> --
> Günther Deschner GPG-ID: 8EE11688
> Red Hat gdeschner at redhat.com
> Samba Team gd at samba.org
> 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
>
More information about the samba-technical
mailing list