From 1f9024d95e5fda5c0afcfb306ad649ab15f06d44 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 11 May 2020 14:10:54 -0700 Subject: [PATCH 1/5] s3: selftest: Add share definition [bad_iconv] in fileserver. Creates a utf8 valid filename within that is invalid in CP850. Useful to test smbclient list directory character set conversions. https://bugzilla.samba.org/show_bug.cgi?id=14374 Signed-off-by: Jeremy Allison --- selftest/target/Samba3.pm | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 083846c87b6..e988ef7210b 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1287,6 +1287,9 @@ sub setup_fileserver my $dropbox_sharedir="$share_dir/dropbox"; push(@dirs,$dropbox_sharedir); + my $bad_iconv_sharedir="$share_dir/bad_iconv"; + push(@dirs, $bad_iconv_sharedir); + my $ip4 = Samba::get_ipv4_addr("FILESERVER"); my $fileserver_options = " kernel change notify = yes @@ -1382,6 +1385,11 @@ sub setup_fileserver writeable = yes vfs objects = +[bad_iconv] + path = $bad_iconv_sharedir + comment = smb username is [%U] + vfs objects = + [homes] comment = Home directories browseable = No @@ -1454,6 +1462,11 @@ sub setup_fileserver ## create_file_chmod("$valid_users_sharedir/foo", 0644) or return undef; + ## + ## create a valid utf8 filename which is invalid as a CP850 conversion + ## + create_file_chmod("$bad_iconv_sharedir/\xED\x9F\xBF", 0644) or return undef; + return $vars; } -- 2.20.1 From 74df46899bedfba545b4986b33bd205712824ee6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 11 May 2020 15:37:00 -0700 Subject: [PATCH 2/5] s3: selftest: Add test_smbclient_iconv.sh to check client behavior on bad name conversion. SMB2 and NT1 fail this, CORE already returns NT_STATUS_INVALID_NETWORK_RESPONSE on bad conversion. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14374 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/bad_iconv | 3 ++ source3/script/tests/test_smbclient_iconv.sh | 53 ++++++++++++++++++++ source3/selftest/tests.py | 9 ++++ 3 files changed, 65 insertions(+) create mode 100644 selftest/knownfail.d/bad_iconv create mode 100755 source3/script/tests/test_smbclient_iconv.sh diff --git a/selftest/knownfail.d/bad_iconv b/selftest/knownfail.d/bad_iconv new file mode 100644 index 00000000000..cdedc70e78b --- /dev/null +++ b/selftest/knownfail.d/bad_iconv @@ -0,0 +1,3 @@ +samba3.blackbox.smbclient_iconv.NT1 +samba3.blackbox.smbclient_iconv.SMB2 + diff --git a/source3/script/tests/test_smbclient_iconv.sh b/source3/script/tests/test_smbclient_iconv.sh new file mode 100755 index 00000000000..0ec7b67dbf7 --- /dev/null +++ b/source3/script/tests/test_smbclient_iconv.sh @@ -0,0 +1,53 @@ +#!/bin/sh + +# This checks directory listing with a file containing +# an invalid CP850 conversion name returns NT_STATUS_INVALID_NETWORK_RESPONSE + +if [ $# -lt 6 ]; then +cat < $smbclient_config < Date: Mon, 11 May 2020 12:34:10 -0700 Subject: [PATCH 3/5] s3: libsmb: In SMB1 old protocol - return NT_STATUS_INVALID_NETWORK_RESPONSE if name conversion ended up with a NULL filename. Can happen if namelen == 0. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14374 Signed-off-by: Jeremy Allison --- source3/libsmb/clilist.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c index f868e72a239..28449dec81c 100644 --- a/source3/libsmb/clilist.c +++ b/source3/libsmb/clilist.c @@ -552,7 +552,10 @@ static NTSTATUS cli_list_old_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, TALLOC_FREE(finfo); return NT_STATUS_NO_MEMORY; } - + if (finfo->name == NULL) { + TALLOC_FREE(finfo); + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } status = is_bad_finfo_name(state->cli, finfo); if (!NT_STATUS_IS_OK(status)) { smbXcli_conn_disconnect(state->cli->conn, status); -- 2.20.1 From 8fffab6975cb5eff53f83f1aa2bb276c283292f6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 11 May 2020 12:23:49 -0700 Subject: [PATCH 4/5] s3: libsmb: In SMB2 return NT_STATUS_INVALID_NETWORK_RESPONSE if name conversion ended up with a NULL filename. Can happen if namelen == 0. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14374 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/bad_iconv | 1 - source3/libsmb/cli_smb2_fnum.c | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail.d/bad_iconv b/selftest/knownfail.d/bad_iconv index cdedc70e78b..c45022f3457 100644 --- a/selftest/knownfail.d/bad_iconv +++ b/selftest/knownfail.d/bad_iconv @@ -1,3 +1,2 @@ samba3.blackbox.smbclient_iconv.NT1 -samba3.blackbox.smbclient_iconv.SMB2 diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index d29341c1708..0622a05a655 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -1269,6 +1269,12 @@ static NTSTATUS parse_finfo_id_both_directory_info(uint8_t *dir_data, /* Bad conversion. */ return NT_STATUS_INVALID_NETWORK_RESPONSE; } + + if (finfo->name == NULL) { + /* Bad conversion. */ + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + return NT_STATUS_OK; } -- 2.20.1 From e635c88e198526d28195a915f78a5efde629cb3d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 11 May 2020 15:58:27 -0700 Subject: [PATCH 5/5] s3: libsmbclient: Finish unifing bad iconv behavior across CORE NT1 SMB2 protocols. On bad name conversion, exit the directory listing with an error, but leave the connection intact. We were already checking for finfo->name == NULL here, but were ignoring it and not reporting an error. Remove the knownfail.d/bad_iconv file as we now behave the same across CORE/NT1/SMB2. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14374 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/bad_iconv | 2 -- source3/libsmb/clilist.c | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-) delete mode 100644 selftest/knownfail.d/bad_iconv diff --git a/selftest/knownfail.d/bad_iconv b/selftest/knownfail.d/bad_iconv deleted file mode 100644 index c45022f3457..00000000000 --- a/selftest/knownfail.d/bad_iconv +++ /dev/null @@ -1,2 +0,0 @@ -samba3.blackbox.smbclient_iconv.NT1 - diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c index 28449dec81c..f9444bc401c 100644 --- a/source3/libsmb/clilist.c +++ b/source3/libsmb/clilist.c @@ -794,8 +794,9 @@ static void cli_list_trans_done(struct tevent_req *subreq) if (finfo->name == NULL) { DEBUG(1, ("cli_list: Error: unable to parse name from " "info level %d\n", state->info_level)); - ff_eos = true; - break; + tevent_req_nterror(req, + NT_STATUS_INVALID_NETWORK_RESPONSE); + return; } status = is_bad_finfo_name(state->cli, finfo); -- 2.20.1