If a client with dialect "NT LM 0.12" sends an SMB_COM_CHECK_DIRECTORY with SMB_FLAGS2_DFS and this path: \x//\/ (with C string escaping, "\\x//\\/"), then this assert fails in openat_pathref_fsp_nosymlink() in source3/smbd/files.c: /* * Path sanitizing further up has cleaned or rejected * empty path components. Assert this here. */ SMB_ASSERT(rel_fname.base_name[0] != '\0'); The function is called with path_in = "/", and its call to path_to_strv() yields empty components. This is in the git source as of a few days ago.
Do you have a test client that can easily reproduce this ? Or did you use smbclient ?
(In reply to Jeremy Allison from comment #1) Also, can you run this against Windows to see what it returns?
Created attachment 17980 [details] program to reproduce
(In reply to Jeremy Allison from comment #1) I've attached a client program that, on my setup, triggers the problem. However, I suspect it may take a lot of work to turn my client code into something that works for others.
(In reply to Ralph Böhme from comment #2) I'm afraid I don't have access to a Windows setup.
(In reply to Robert Morris from comment #4) Well it compiles easily enough :-). So I should be able to run this against a local Samba instance with no security once I get back from Portland next week. Thanks for sending this in !
Ah. Found the problem. You're using our SMB1 DFS path code processing...
Created attachment 17981 [details] Patch for master. Clever attack. It uses the fact that SMB1 DFS processing swallows the "server" and "share" components without looking at them - separated by '\\' characters, and the fact that our SMB1 code is designed to convert '\\' DFS separators into '/' before passing them to the "strip DFS server and share components" code. That allows you to tunnel UNIX path components inside the SMB1 DFS server and share names, and because we only remove the first 2 matching components starting with the '/' character, any others you can add get passed into the pathname walk functions without them going through check_path_syntax() first. This won't work against SMB2 as we explicitly only look at and remove the first two '\\' separated components, and any '/' characters hidden in the server and sharenames will get removed with them. The rest always gets passed to check_path_syntax() explicitly. This patch fixes the problem for me. I'll work on adding a regression test to our SMB1 DFS test code.
Actually I think I'm going to harden this some more by converting both '/' and '\\' characters in the server and sharename components to '_''s, just in case I've missed a code path where we convert '\\' to '/' on the full passed in client path. As the server and share components of the DFS path are ignored, it won't matter to any valid codepaths.
Created attachment 17982 [details] Patch for master Converts both '/' and '\\' characters in servername and sharename for SMB1 DFS paths to '_' characters.
Robert, can you confirm this patch fixes the issue for you ? Thanks !
(In reply to Jeremy Allison from comment #11) Yes, the patch fixes the problem for me.
Got a build with a regression test cooking in gitlab. I'll update with the MR if it passes ci.
https://gitlab.com/samba-team/samba/-/merge_requests/3197
This bug was referenced in samba master: 2aa9ffa2f0fc79599efbfe0c37aac4ef5160f712 20df26b908182f0455f301a51aeb54b6044af580
Created attachment 18004 [details] git-am fix for 4.18.next Cherry-picked from master.
Created attachment 18005 [details] git-am fix for 4.17.next Back-ported from master. NB. The SMB1 DFS test infrastructure doesn't exist in 4.17 so this is just the server fix code.
This bug was referenced in samba v4-19-test: 2aa9ffa2f0fc79599efbfe0c37aac4ef5160f712 20df26b908182f0455f301a51aeb54b6044af580
This bug was referenced in samba v4-19-stable (Release samba-4.19.0rc1): 2aa9ffa2f0fc79599efbfe0c37aac4ef5160f712 20df26b908182f0455f301a51aeb54b6044af580
Jule, can we get the 4.18 and 4.17 patches into the next releases ? Thanks ! Jeremy.
Pushed to autobuild-v4-{18,17}-test.
This bug was referenced in samba v4-17-test: fec913830f5e03c221ad08011f79a0ae8eb1eddf
Hi Jule, I don't see this one in 4.18.6. It did get into the v4-17-test branch, just not the v4-18-test branch. Can we get it in for the 4.18.7 release please ?
The autobuild for 4.18 failed several times for tasks samba-fileserver and samba-no-opath2: UNEXPECTED(failure): samba3.smbtorture_s3.smb1.SMB1-DFS-BADPATH.smbtorture(fileserver_smb1) REASON: Exception: Exception: using seed 1692358048 host=10.53.57.53 share=msdfs-pathname-share user=janger myname=fileserversmb1 Running SMB1-DFS-BADPATH Starting SMB1-DFS-CHECK-BADPATH ../../source3/torture/test_smb1_dfs.c:3829 SMB1chkpath of \x//\/ failed (NT_STATUS_OBJECT_PATH_NOT_FOUND) TEST SMB1-DFS-BADPATH FAILED! SMB1-DFS-BADPATH took 0.025272 secs Sorry for not informing you sooner.
Created attachment 18064 [details] git-am fix for 4.18.next Looks like for this specific non-functional DFS path "\\x//\\/" in SMB1 the 4.18.x SMB1 server returns NT_STATUS_OBJECT_PATH_NOT_FOUND instead of NT_STATUS_OK (as in 4.19 and master). Easiest thing is to change the test to look for NT_STATUS_OBJECT_PATH_NOT_FOUND instead of NT_STATUS_OK, as this is a DFS path that should map to "\X_\_" rather than trying to change the SMB1 server behavior. We're only checking that a path contining '/' characters doesn't get through the DFS \server\share checks anyway. Sorry Jule for not checking earlier.
(In reply to Jeremy Allison from comment #25) It also explains why the 4.17 fix wasn't affected (I'm sure that smbd returns NT_STATUS_OBJECT_PATH_NOT_FOUND in this case also) - the test wasn't back-ported for that release as the test framework isn't there :-). As I said, it's a corner case - in 4.17,4.18 I think the server checks the \server\share names are correct for the actual server of the DFS share and so returns NT_STATUS_OBJECT_PATH_NOT_FOUND. 4.19,master follow the Windows server behavior in that *any* strings are allowed in the \SERVER and \SHARE DFS name components so it doesn't check them and so \BAD\NAME just maps to root of share (an empty "" pathname).
Comment on attachment 18064 [details] git-am fix for 4.18.next Ping Ralph, can I get this reviewed for 4.18.next ?
Reassigning to Jule for inclusion in 4.18.
This bug was referenced in samba v4-18-test: b80fdc0b0b3d637afd9871493c5813916cef2f40 fd1111c2f48db0c90f430c4a3d1636d7973494d3
Closing out bug report. Thanks!
This bug was referenced in samba v4-17-stable (Release samba-4.17.11): fec913830f5e03c221ad08011f79a0ae8eb1eddf
This bug was referenced in samba v4-18-stable (Release samba-4.18.7): b80fdc0b0b3d637afd9871493c5813916cef2f40 fd1111c2f48db0c90f430c4a3d1636d7973494d3