Currently smbclient segfaults when marshalling an ntlmssp NEGOTIATE_MESSAGE if a wide character (e.g. Cyrillic) netbios name or workgroup is configured in smb.conf. /etc/samba/smb.conf: ... [global] netbios name = нФЏБа ... Core was generated by `/opt/samba/bin/smbclient -N -L 10.10.101.248'. Program terminated with signal 11, Segmentation fault. #0 0x00007f76fc7e05ab in memcpy () from /lib64/libc.so.6 (gdb) bt #0 0x00007f76fc7e05ab in memcpy () from /lib64/libc.so.6 #1 0x0000000000525f9a in msrpc_gen (mem_ctx=0xe7dea0, blob=0x7fff849524b0, format=0xa6ee32 "CddAA") at ../libcli/auth/msrpc_parse.c:156 #2 0x000000000051024b in ntlmssp_client_initial (ntlmssp_state=0xe7dea0, out_mem_ctx=0xe7dea0, reply=..., next_request=0x7fff849524b0) at libsmb/ntlmssp.c:391 #3 0x000000000050febc in ntlmssp_update (ntlmssp_state=0xe7dea0, input=..., out=0x7fff849524b0) at libsmb/ntlmssp.c:268 #4 0x00000000004caf00 in cli_session_setup_ntlmssp_send (mem_ctx=0xe7f350, ev=0xe7f350, cli=0xe5d5b0, user=0xe7e8b0 'E' <repeats 41 times>"\200, ", pass=0xe52cc0 "", domain=0xe4c120 "WORKGROUP") at libsmb/cliconnect.c:1034 #5 0x00000000004cb6d1 in cli_session_setup_ntlmssp (cli=0xe5d5b0, user=0xe7e8b0 'E' <repeats 41 times>"\200, ", pass=0xe52cc0 "", domain=0xe4c120 "WORKGROUP") at libsmb/cliconnect.c:1178 #6 0x00000000004cc17f in cli_session_setup_spnego (cli=0xe5d5b0, user=0xe52c40 "root", pass=0xe52cc0 "", user_domain=0xe4c120 "WORKGROUP", dest_realm=0x0) at libsmb/cliconnect.c:1354 #7 0x00000000004cc57e in cli_session_setup (cli=0xe5d5b0, user=0xe52c40 "root", pass=0xe52cc0 "", passlen=0, ntpass=0xe52cc0 "", ntpasslen=0, workgroup=0xe4c120 "WORKGROUP") at libsmb/cliconnect.c:1446 #8 0x00000000004faf4f in do_connect (ctx=0xe4c0f0, server=0xe52ba0 "10.10.101.248", share=0xa4d370 "IPC$", auth_info=0xe52740, show_sessetup=true, force_encrypt=false, max_protocol=5, port=0, name_type=32) at libsmb/clidfs.c:188 #9 0x00000000004fb584 in cli_cm_connect (ctx=0xe4c0f0, referring_cli=0x0, server=0xe52ba0 "10.10.101.248", share=0xa4d370 "IPC$", auth_info=0xe52740, show_hdr=true, force_encrypt=false, max_protocol=5, port=0, name_type=32) at libsmb/clidfs.c:307 #10 0x00000000004fb90e in cli_cm_open (ctx=0xe4c0f0, referring_cli=0x0, server=0xe52ba0 "10.10.101.248", share=0xa4d370 "IPC$", auth_info=0xe52740, show_hdr=true, force_encrypt=false, max_protocol=5, port=0, name_type=32) at libsmb/clidfs.c:401 #11 0x00000000004821b8 in do_host_query (query_host=0xe52ba0 "10.10.101.248") at client/client.c:4942 #12 0x00000000004835ae in main (argc=4, argv=0x7fff849532b8) at client/client.c:5369 push_ascii_talloc() fails to marshall the UTF netbios_name and returns an empty string and zero length, return status is successful due to the allow_bad_conv=true argument passed to the convert_string_talloc() helper function. On success as a null terminator is assumed to be present and included in the length value. The zero length returned in this case is decremented which causes the value to wrap. The length is then used for a memcopy operation.
A proposed fix has been sent to samba-technical: http://lists.samba.org/archive/samba-technical/2011-March/076937.html
Created attachment 6346 [details] git-am fix for 3.5.next Here's the patch I added to v3-6-test. I think it fixes the root cause of the problem rather than patching the msrpc_gen() code. David - can you test this please ? Thanks, Jeremy.
(In reply to comment #2) > Created attachment 6346 [details] > git-am fix for 3.5.next > > Here's the patch I added to v3-6-test. I think it fixes the root cause of the > problem rather than patching the msrpc_gen() code. David - can you test this > please ? The fix looks sane Jeremy, however a commit to cleanup convert_string_talloc() (15e84a9) which was pushed in the meantime uncovers further issues. smbclient and smbd now panic in unix_strlower() following convert_string_talloc() failure. set_local_machine_name() calls alpha_strcpy(), which from what I can gather is supposed to replace any characters not part of A-z0-9 or SAFE_NETBIOS_CHARS. This does not occur for the netbios name ТАНЦЕВАТЬ. I'm currently testing with the following additional changes: From 200dea0d11d76e04cdf495e9a1e127255a330265 Mon Sep 17 00:00:00 2001 From: David Disseldorp <ddiss@suse.de> Date: Sun, 27 Mar 2011 19:26:08 +0200 Subject: [PATCH 1/2] s3-client: check for failures generating ntlmssp negotiate Since commit 15e84a9 was pushed to cleanup convert_string_talloc(), character conversion errors may propagate up to msrpc_gen() when a wide-character netbios name or workgroup is configured. These errors should not be ignored to avoid sending malformed ntlmssp negotiate requests. --- source3/libsmb/ntlmssp.c | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/source3/libsmb/ntlmssp.c b/source3/libsmb/ntlmssp.c index 7a006a3..9251cb7 100644 --- a/source3/libsmb/ntlmssp.c +++ b/source3/libsmb/ntlmssp.c @@ -377,6 +377,8 @@ static NTSTATUS ntlmssp_client_initial(struct ntlmssp_state *ntlmssp_state, TALLOC_CTX *out_mem_ctx, /* Unused at this time */ DATA_BLOB reply, DATA_BLOB *next_request) { + bool ok; + if (ntlmssp_state->unicode) { ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_UNICODE; } else { @@ -388,12 +390,16 @@ static NTSTATUS ntlmssp_client_initial(struct ntlmssp_state *ntlmssp_state, } /* generate the ntlmssp negotiate packet */ - msrpc_gen(ntlmssp_state, next_request, "CddAA", - "NTLMSSP", - NTLMSSP_NEGOTIATE, - ntlmssp_state->neg_flags, - ntlmssp_state->client.netbios_domain, - ntlmssp_state->client.netbios_name); + ok = msrpc_gen(ntlmssp_state, next_request, "CddAA", + "NTLMSSP", + NTLMSSP_NEGOTIATE, + ntlmssp_state->neg_flags, + ntlmssp_state->client.netbios_domain, + ntlmssp_state->client.netbios_name); + if (!ok) { + DEBUG(0, ("failed to generate ntlmssp negotiate packet\n")); + return NT_STATUS_INVALID_PARAMETER; + } if (DEBUGLEVEL >= 10) { struct NEGOTIATE_MESSAGE *negotiate = talloc( -- 1.7.3.4 From cee29a9d7f1cbd9a35b0e91c58ec1f2e46c62604 Mon Sep 17 00:00:00 2001 From: David Disseldorp <ddiss@suse.de> Date: Sun, 27 Mar 2011 20:07:54 +0200 Subject: [PATCH 2/2] s3-charcnv: use isascii() in alpha_strcpy() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit alpha_strcpy() is a utility function which reportedly: Strips out all but 'a-Z0-9' and the character in other_safe_chars and replaces with '_'. This statement does not currently hold true in all cases (e.g. src = "ТАНЦЕВАТЬ"). Use C99 isascii() instead of the isupper_ascii() islower_ascii() helpers to resolve this. --- source3/lib/util_str.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/lib/util_str.c b/source3/lib/util_str.c index 1996174..b7e8d9a 100644 --- a/source3/lib/util_str.c +++ b/source3/lib/util_str.c @@ -534,11 +534,12 @@ char *alpha_strcpy(char *dest, for(i = 0; i < len; i++) { int val = (src[i] & 0xff); - if (isupper_ascii(val) || islower_ascii(val) || - isdigit(val) || strchr_m(other_safe_chars, val)) + if ((isascii(val) && isalnum(val)) + || strchr_m(other_safe_chars, val)) { dest[i] = src[i]; - else + } else { dest[i] = '_'; + } } dest[i] = '\0'; -- 1.7.3.4
The following changes since commit b47b4c9e3dd1a3776f38b879041fb83a0edb6c36: s3: Remove two calls to cli_errstr (2011-03-27 13:17:47 +0200) are available in the git repository at: git://oss.sgi.com/ddiss/samba/ bnc680169-smbclient-segv-rb2 David Disseldorp (2): s3-client: check for failures generating ntlmssp negotiate s3-charcnv: use isascii() in alpha_strcpy() source3/lib/util_str.c | 7 ++++--- source3/libsmb/ntlmssp.c | 18 ++++++++++++------ 2 files changed, 16 insertions(+), 9 deletions(-)
Ok from this comment : "The fix looks sane Jeremy, however a commit to cleanup convert_string_talloc() (15e84a9) which was pushed in the meantime uncovers further issues." This fix (15e84a9) only went into master, so I'm assuming the alpha_strcpy() change is only for master (4.0.x) - yes ? I'm going to adapt your msrpc_gen fix for 3.5.x and add this to as an additional patch to the bug report. Can you confirm this is then fixed in 3.5.next with the two fixes added as attachments ? Jeremy.
Hi Jeremy, (In reply to comment #5) ... > I'm going to adapt your msrpc_gen fix for 3.5.x and add this to as an > additional patch to the bug report. Can you confirm this is then fixed in > 3.5.next with the two fixes added as attachments ? > I've tested against v3-5-test with your fix (00834d0) cherry-picked, the results were successful. Sorry for the delay. Cheers, David
Created attachment 6361 [details] First part of fix. Volker, both fixes are needed for this bug. Once you've ack'ed please re-assign to ks for inclusion in 3.5.next. Thanks ! Jeremy.
Comment on attachment 6361 [details] First part of fix. Sorry - mixed up the two patches here. Wait until I've sorted out the second patch... Jeremy.
Created attachment 6362 [details] Second part of fix. Ok, finally got part #1 and part #2 correctly sorted out. They're both git-am patches. Jeremy.
Pushed both patches to v3-5-test. Closing out bug. Thanks!