Windows OpenPrinterEx requests have been observed carrying a device mode formname "A4" followed by non-utf16 garbage after the null terminator. Such requests fail during unmarshalling in the ndr_pull_charset() codepath, causing print job failures. Currently the entire device_mode->formname array is processed for character conversion during unmarshalling. This is unnecessary, only bytes prior to the null terminator need be converted.
Can you upload some binary examples of this ?
(In reply to comment #1) > Can you upload some binary examples of this ? I should be able to, I'll need to get permission from the customer first. In the meantime I've pushed a spoolss torture test and proposed fix to: http://git.samba.org/?p=ddiss/samba.git;a=shortlog;h=refs/heads/formname_char_conv_rb2 The following changes since commit 72cabbbe50a36986dde823f0ba60abf9052c535a: s3:smb2_flush: outbody only needs 4 bytes (2011-11-14 10:01:30 +0100) are available in the git repository at: git://git.samba.org/ddiss/samba.git formname_char_conv_rb2 David Disseldorp (5): spoolss: test unmarshalling formname with garbage after null ndr: add ndr_pull_charset_to_null() idl: add to_null property idl: add parser for the to_null property idl: add to_null attribute to the spoolss formname array librpc/idl/spoolss.idl | 2 +- librpc/ndr/libndr.h | 1 + librpc/ndr/ndr_string.c | 31 ++++++++++++++++++++++++++++++ pidl/lib/Parse/Pidl/NDR.pm | 9 +++++++- pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 6 ++++- source4/torture/rpc/spoolss.c | 28 +++++++++++++++++++++++++++ 6 files changed, 74 insertions(+), 3 deletions(-)
Below is an example of garbage after the OpenPrinterEx->DeviceMode->formname and OpenPrinterEx->DeviceMode->devicename null terminators: [2011/11/22 14:05:13.089754, 10, pid=28264] ../lib/util/util.c:304(_dump_data) [0000] 00 5C 00 50 00 49 00 50 00 45 00 5C 00 00 00 00 .\.P.I.P .E.\.... [0010] 00 05 00 00 03 10 00 00 00 9E 04 00 00 01 00 00 ........ ........ [0020] 00 86 04 00 00 00 00 45 00 00 00 02 00 14 00 00 .......E ........ [0030] 00 00 00 00 00 14 00 00 00 5C 00 5C 00 59 00 47 ........ .\.\.Y.G [0040] 00 34 00 52 00 39 00 30 00 31 00 33 00 5C 00 4E .4.R.9.0 .1.3.\.N [0050] 00 57 00 44 00 5F 00 34 00 38 00 34 00 31 00 00 .W.D._.4 .8.4.1.. [0060] 00 04 00 02 00 04 00 00 00 00 00 00 00 04 00 00 ........ ........ [0070] 00 52 00 41 00 57 00 00 00 B4 03 00 00 08 00 02 .R.A.W.. ........ [0080] 00 B4 03 00 00 5C 00 5C 00 59 00 47 00 34 00 52 .....\.\ .Y.G.4.R [0090] 00 39 00 30 00 31 00 33 00 5C 00 4E 00 57 00 44 .9.0.1.3 .\.N.W.D [00A0] 00 5F 00 34 00 38 00 34 00 31 00 00 00 A0 00 BA ._.4.8.4 .1...... * garbage follows here... [00B0] 00 92 01 02 00 38 00 CA 00 92 01 02 00 C8 00 AA .....8.. ........ [00C0] 00 92 01 02 00 01 04 02 05 DC 00 D8 02 53 FF 81 ........ .....S.. [00D0] 03 01 00 09 00 9A 0B 34 08 64 00 01 00 0F 00 58 .......4 .d.....X [00E0] 02 02 00 01 00 58 02 02 00 01 00 41 00 34 00 00 .....X.. ...A.4.. * and here... [00F0] 00 CB 00 92 01 02 00 78 00 13 20 92 01 02 00 F0 .......x .. ..... [0100] 00 1E 20 92 01 02 00 18 00 22 20 92 01 02 00 E0 .. ..... ." ..... [0110] 00 B0 00 92 01 02 00 68 00 AA 00 92 01 02 00 E0 .......h ........ [0120] 00 92 01 92 01 02 00 38 00 C5 00 00 00 00 00 00 .......8 ........ [0130] 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 ........ ........ [0140] 00 01 00 00 00 02 00 00 00 00 01 00 00 00 00 00 ........ ........ [0150] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........ ........ [0160] 00 50 52 49 56 E2 30 00 00 00 00 00 00 00 00 00 .PRIV.0. ........ [0170] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........ ........ [0180] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........ ........ [0190] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........ ........ [01A0] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........ ........ [01B0] 00 00 00 00 00 00 00 00 00 18 00 00 00 00 00 10 ........ ........ [01C0] 27 10 27 10 27 00 00 10 27 00 00 00 00 00 00 00 '.'.'... '....... [01D0] 00 00 00 C4 02 00 00 00 00 00 00 00 00 00 00 00 ........ ........ [01E0] 00 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00 ........ ........ [01F0] 00 00 00 00 00 14 00 10 00 50 34 03 00 28 88 04 ........ .P4..(..
Changes since comment 2: - add to_null attribute to the spoolss devicename array - tweak the existing spoolss ndr torture test, rather than the writing new rpc tests to do the same thing. The following changes since commit 3741cf999e0f05a831d88330ac6bfa7ad34c2ec7: Remove unused variable. (2011-11-24 00:17:40 +0100) are available in the git repository at: git://git.samba.org/ddiss/samba.git gd_formname_ndr_test_with_fix_rb1 David Disseldorp (6): s4-smbtorture: tweak spoolss_OpenPrinterEx devmode idl: add parser for the to_null property idl: add to_null property ndr: add ndr_pull_charset_to_null() idl: add to_null attribute to the spoolss formname array idl: add to_null attribute to the spoolss devicename array librpc/idl/spoolss.idl | 4 +- librpc/ndr/libndr.h | 1 + librpc/ndr/ndr_string.c | 31 ++++++++++++++++++++++++++++++ pidl/lib/Parse/Pidl/NDR.pm | 9 +++++++- pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 6 ++++- source4/torture/ndr/spoolss.c | 4 +- 6 files changed, 49 insertions(+), 6 deletions(-)
Ok, this patchset: http://git.samba.org/?p=ddiss/samba.git;a=shortlog;h=refs/heads/gd_formname_ndr_test_with_fix_rb1 looks correct to me for 3.6.next. What is the status of this fix ? I'm adding Metze to the CC: list as I recall him making a comment on this. Jeremy.
Created attachment 7240 [details] git-am fix for 3.6.next David's patch back-ported for 3.5.next. Jeremy.
Comment on attachment 7240 [details] git-am fix for 3.6.next Arg. I meant "back ported for 3.6.next", not 3.5.next.
Comment on attachment 7240 [details] git-am fix for 3.6.next Back-port looks good, thanks Jeremy!
Re-assigning to Karolin for inclusion in 3.6.next. Jeremy.
Pushed to v3-6-test. Closing out bug report. Thanks!