From 4f569f8ac4b8fd03d99f60520b647b3ec3609414 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 4 Dec 2014 20:03:39 +0100 Subject: [PATCH 1/7] spoolss: clear JobInfo on GetJob error In handling a spoolss GetJob request, the _spoolss_GetJob() handler may return an immediate error if one of the input parameters is invalid. If this is done without zeroing the pre-allocated @info pointer, then api_spoolss_GetJob() will attempt to marshall @info, which in the case of an @offered value of zero results in a marshalling error: ndr_push_error(7): Bad subcontext (PUSH) content_size 64 is larger than size_is(0) Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 Signed-off-by: David Disseldorp --- source3/rpc_server/spoolss/srv_spoolss_nt.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index 1226ec1..c71eb91 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -9484,7 +9484,8 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, /* that's an [in out] buffer */ if (!r->in.buffer && (r->in.offered != 0)) { - return WERR_INVALID_PARAM; + result = WERR_INVALID_PARAM; + goto err_jinfo_free; } DEBUG(5,("_spoolss_GetJob\n")); @@ -9492,12 +9493,14 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, *r->out.needed = 0; if (!get_printer_snum(p, r->in.handle, &snum, NULL)) { - return WERR_BADFID; + result = WERR_BADFID; + goto err_jinfo_free; } svc_name = lp_const_servicename(snum); if (svc_name == NULL) { - return WERR_INVALID_PARAM; + result = WERR_INVALID_PARAM; + goto err_jinfo_free; } result = winreg_get_printer_internal(p->mem_ctx, @@ -9506,22 +9509,22 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, svc_name, &pinfo2); if (!W_ERROR_IS_OK(result)) { - return result; + goto err_jinfo_free; } pdb = get_print_db_byname(svc_name); if (pdb == NULL) { DEBUG(3, ("failed to get print db for svc %s\n", svc_name)); - TALLOC_FREE(pinfo2); - return WERR_INVALID_PARAM; + result = WERR_INVALID_PARAM; + goto err_pinfo_free; } sysjob = jobid_to_sysjob_pdb(pdb, r->in.job_id); release_print_db(pdb); if (sysjob == -1) { DEBUG(3, ("no sysjob for spoolss jobid %u\n", r->in.job_id)); - TALLOC_FREE(pinfo2); - return WERR_INVALID_PARAM; + result = WERR_INVALID_PARAM; + goto err_pinfo_free; } count = print_queue_status(p->msg_ctx, snum, &queue, &prt_status); @@ -9551,8 +9554,7 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, TALLOC_FREE(pinfo2); if (!W_ERROR_IS_OK(result)) { - TALLOC_FREE(r->out.info); - return result; + goto err_jinfo_free; } *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_JobInfo, r->out.info, @@ -9560,6 +9562,12 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); + +err_pinfo_free: + TALLOC_FREE(pinfo2); +err_jinfo_free: + TALLOC_FREE(r->out.info); + return result; } /**************************************************************** -- 2.1.2 From 203a02fdf245714fc37c7834e691de052608db79 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 17 Dec 2014 15:21:33 +0100 Subject: [PATCH 2/7] spoolss: clear DriverInfo on GetPrinterDriver2 error In handling a spoolss GetPrinterDriver2 request, the handler may return an immediate error if one of the input parameters is invalid. If this is done without zeroing the pre-allocated @info pointer, then marshalling of the response will fail. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 Signed-off-by: David Disseldorp --- source3/rpc_server/spoolss/srv_spoolss_nt.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index c71eb91..9023ab6 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -5686,14 +5686,16 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, /* that's an [in out] buffer */ if (!r->in.buffer && (r->in.offered != 0)) { - return WERR_INVALID_PARAM; + result = WERR_INVALID_PARAM; + goto err_info_free; } DEBUG(4,("_spoolss_GetPrinterDriver2\n")); if (!(printer = find_printer_index_by_hnd(p, r->in.handle))) { DEBUG(0,("_spoolss_GetPrinterDriver2: invalid printer handle!\n")); - return WERR_INVALID_PRINTER_NAME; + result = WERR_INVALID_PRINTER_NAME; + goto err_info_free; } *r->out.needed = 0; @@ -5701,7 +5703,8 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, *r->out.server_minor_version = 0; if (!get_printer_snum(p, r->in.handle, &snum, NULL)) { - return WERR_BADFID; + result = WERR_BADFID; + goto err_info_free; } if (r->in.client_major_version == SPOOLSS_DRIVER_VERSION_2012) { @@ -5718,8 +5721,7 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, r->in.architecture, version); if (!W_ERROR_IS_OK(result)) { - TALLOC_FREE(r->out.info); - return result; + goto err_info_free; } *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_DriverInfo, @@ -5727,6 +5729,10 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); + +err_info_free: + TALLOC_FREE(r->out.info); + return result; } -- 2.1.2 From d6c43a21cde1b2b10af10d72646fe09f8db430cd Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 17 Dec 2014 15:29:52 +0100 Subject: [PATCH 3/7] spoolss: clear FormInfo on GetForm error In handling a spoolss GetForm request, the handler may return an immediate error if one of the input parameters is invalid. If this is done without zeroing the pre-allocated @info pointer, then marshalling of the response will fail. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 Signed-off-by: David Disseldorp --- source3/rpc_server/spoolss/srv_spoolss_nt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index 9023ab6..9b898d0 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -7853,6 +7853,7 @@ WERROR _spoolss_GetForm(struct pipes_struct *p, /* that's an [in out] buffer */ if (!r->in.buffer && (r->in.offered != 0)) { + TALLOC_FREE(r->out.info); return WERR_INVALID_PARAM; } -- 2.1.2 From 58a23f7efe903586a1dacce9225c2d1af7d86cd7 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 17 Dec 2014 15:54:22 +0100 Subject: [PATCH 4/7] spoolss: clear info on GetPrintProcessorDirectory error If an error is returned without zeroing a pre-allocated @info pointer, then marshalling of the response will fail. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 Signed-off-by: David Disseldorp --- source3/rpc_server/spoolss/srv_spoolss_nt.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index 9b898d0..c8c670b 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -10146,7 +10146,8 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, /* that's an [in out] buffer */ if (!r->in.buffer && (r->in.offered != 0)) { - return WERR_INVALID_PARAM; + result = WERR_INVALID_PARAM; + goto err_info_free; } DEBUG(5,("_spoolss_GetPrintProcessorDirectory: level %d\n", @@ -10162,7 +10163,8 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, snum = find_service(talloc_tos(), "prnproc$", &prnproc_share); if (!prnproc_share) { - return WERR_NOMEM; + result = WERR_NOMEM; + goto err_info_free; } if (snum != -1) { prnproc_share_exists = true; @@ -10173,8 +10175,7 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, r->in.environment, &r->out.info->info1); if (!W_ERROR_IS_OK(result)) { - TALLOC_FREE(r->out.info); - return result; + goto err_info_free; } *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_PrintProcessorDirectoryInfo, @@ -10182,6 +10183,10 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); + +err_info_free: + TALLOC_FREE(r->out.info); + return result; } /******************************************************************* -- 2.1.2 From 2c80570d35393d1568ac83bf98587b75083afa45 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 17 Dec 2014 16:47:50 +0100 Subject: [PATCH 5/7] spoolss: clear info on GetPrinterDriverDirectory error If an error is returned without zeroing a pre-allocated @info pointer, then marshalling of the response will fail. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 Signed-off-by: David Disseldorp --- source3/rpc_server/spoolss/srv_spoolss_nt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index c8c670b..c34b04d 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -8544,6 +8544,7 @@ WERROR _spoolss_GetPrinterDriverDirectory(struct pipes_struct *p, /* that's an [in out] buffer */ if (!r->in.buffer && (r->in.offered != 0)) { + TALLOC_FREE(r->out.info); return WERR_INVALID_PARAM; } -- 2.1.2 From 482d8e80ab64d30141e64b608c7bbb3083851dd2 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 17 Dec 2014 16:54:42 +0100 Subject: [PATCH 6/7] spoolss: clear PrinterInfo on GetPrinter error If an error is returned without zeroing a pre-allocated @info pointer, then marshalling of the response will fail. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 Signed-off-by: David Disseldorp --- source3/rpc_server/spoolss/srv_spoolss_nt.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index c34b04d..115af2d 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -4778,17 +4778,20 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, /* that's an [in out] buffer */ if (!r->in.buffer && (r->in.offered != 0)) { - return WERR_INVALID_PARAM; + result = WERR_INVALID_PARAM; + goto err_info_free; } *r->out.needed = 0; if (Printer == NULL) { - return WERR_BADFID; + result = WERR_BADFID; + goto err_info_free; } if (!get_printer_snum(p, r->in.handle, &snum, NULL)) { - return WERR_BADFID; + result = WERR_BADFID; + goto err_info_free; } result = winreg_get_printer_internal(p->mem_ctx, @@ -4797,7 +4800,7 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, lp_const_servicename(snum), &info2); if (!W_ERROR_IS_OK(result)) { - goto out; + goto err_info_free; } switch (r->in.level) { @@ -4857,12 +4860,10 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, } TALLOC_FREE(info2); - out: if (!W_ERROR_IS_OK(result)) { DEBUG(0, ("_spoolss_GetPrinter: failed to construct printer info level %d - %s\n", r->in.level, win_errstr(result))); - TALLOC_FREE(r->out.info); - return result; + goto err_info_free; } *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_PrinterInfo, @@ -4870,6 +4871,10 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); + +err_info_free: + TALLOC_FREE(r->out.info); + return result; } /******************************************************************** -- 2.1.2 From f1d89e3df4448885b0c270427d45fcb25f0d733f Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 3 Dec 2014 18:44:37 +0100 Subject: [PATCH 7/7] torture/spoolss: issue GetJob after StartDocPrinter This reflects Windows XP spoolss client behaviour. This fails if the job is not yet instantiated on the server, and prior to the bso#10984 fix resulted in an unsable DCERPC pipe. Signed-off-by: David Disseldorp --- source4/torture/rpc/spoolss.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c index 20e86fc..23f501d 100644 --- a/source4/torture/rpc/spoolss.c +++ b/source4/torture/rpc/spoolss.c @@ -3446,6 +3446,9 @@ static bool test_DoPrintTest_add_one_job_common(struct torture_context *tctx, torture_assert_werr_ok(tctx, s.out.result, "StartDocPrinter failed"); for (i=1; i < 4; i++) { + union spoolss_JobInfo ginfo; + bool ok; + torture_comment(tctx, "Testing StartPagePrinter: Page[%d], JobId[%d]\n", i, *job_id); sp.in.handle = handle; @@ -3455,6 +3458,11 @@ static bool test_DoPrintTest_add_one_job_common(struct torture_context *tctx, "dcerpc_spoolss_StartPagePrinter failed"); torture_assert_werr_ok(tctx, sp.out.result, "StartPagePrinter failed"); + ok = test_GetJob_args(tctx, b, handle, *job_id, 1, &ginfo); + if (!ok) { + torture_comment(tctx, "test_GetJob failed for JobId[%d]\n", *job_id); + } + torture_comment(tctx, "Testing WritePrinter: Page[%d], JobId[%d]\n", i, *job_id); w.in.handle = handle; -- 2.1.2