From 2f33284f78ff94e305e2bcb5003969daa360f593 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 Reviewed-by: Andreas Schneider (back-ported from commit a9651d6bc2b6dea8adc859ce21c2431253868887) --- selftest/target/Samba3.pm | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index cdbbbdcef3d..1bfb72af690 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -959,6 +959,9 @@ sub setup_fileserver my $usershare_sharedir="$share_dir/usershares"; push(@dirs,$usershare_sharedir); + my $bad_iconv_sharedir="$share_dir/bad_iconv"; + push(@dirs, $bad_iconv_sharedir); + my $fileserver_options = " kernel change notify = yes rpc_server:mdssvc = embedded @@ -1039,6 +1042,12 @@ sub setup_fileserver path = $share_dir comment = force group test # force group = everyone + +[bad_iconv] + path = $bad_iconv_sharedir + comment = smb username is [%U] + vfs objects = + [homes] comment = Home directories browseable = No @@ -1107,6 +1116,17 @@ sub setup_fileserver close(VALID_USERS_TARGET); chmod 0644, $valid_users_target; + ## + ## create a valid utf8 filename which is invalid as a CP850 conversion + ## + my $bad_iconv_target = "$bad_iconv_sharedir/\xED\x9F\xBF"; + unless (open(BAD_ICONV_TARGET, ">$bad_iconv_target")) { + warn("Unable to open $bad_iconv_target"); + return undef; + } + close(BAD_ICONV_TARGET); + chmod 0644, $bad_iconv_target; + return $vars; } -- 2.20.1 From 482ee59b0f1955ce0883802337d43fc2c55637cb 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 Reviewed-by: Andreas Schneider (back-ported from commit e016671d34c24c4768df774425ec743b88e30015) --- selftest/knownfail.d/bad_iconv | 3 ++ source3/script/tests/test_smbclient_iconv.sh | 53 ++++++++++++++++++++ source3/selftest/tests.py | 6 +++ 3 files changed, 62 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 Reviewed-by: Andreas Schneider (cherry picked from commit b10de0bb64fe022e6b066584013dfb0bdf2ade96) --- 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 3be4401cacac91e7f9e59ba3a0fcf1cc3184c1e5 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 Reviewed-by: Andreas Schneider (cherry picked from commit 753115a8d19f6ac8cd28305748fc6d888679dccc) --- 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 4cae87853db..8c8b33f49ed 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 fa11dc4ecf9d97c49cf01530857ff6024ade2977 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 Reviewed-by: Andreas Schneider Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue May 12 21:32:44 UTC 2020 on sn-devel-184 (cherry picked from commit 393da520e43bd3a28feb231bcd9fd5308a3daa4a) --- 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