Bug 13093 - smbclient doesn't correctly canonicalize all local names before use.
smbclient doesn't correctly canonicalize all local names before use.
Status: ASSIGNED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient
unspecified
All All
: P5 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-10-20 21:31 UTC by Jeremy Allison
Modified: 2017-11-13 11:43 UTC (History)
3 users (show)

See Also:


Attachments
git-am fix for master (22.78 KB, patch)
2017-10-20 22:22 UTC, Jeremy Allison
no flags Details
Updated git-am fix for master. (27.06 KB, patch)
2017-10-20 23:26 UTC, Jeremy Allison
no flags Details
Updated git-am fix for master. (17.87 KB, patch)
2017-10-21 00:16 UTC, Jeremy Allison
no flags Details
Updated git-am fix for master. (20.04 KB, patch)
2017-10-23 22:44 UTC, Jeremy Allison
asn: review+
Details
git-am backport for 4.7.next (20.26 KB, patch)
2017-10-24 23:22 UTC, Jeremy Allison
jra: review? (asn)
asn: review+
Details
git-am fix for 4.6.next. (19.80 KB, patch)
2017-10-24 23:50 UTC, Jeremy Allison
jra: review? (asn)
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2017-10-20 21:31:34 UTC
Actually a smbclient bug, not a libsmbclient bug.

We're missing calls to clean_name() after assembling paths.
Comment 1 Jeremy Allison 2017-10-20 21:32:47 UTC
Found by Andreas by doing:

rename FILE ..\newdir\FILE

over SMB2. We got away with it in SMB1 as in SMB1 the server will canonicalize these paths (and smbd does it for SMB2 also).
Comment 2 Jeremy Allison 2017-10-20 21:33:04 UTC
That's smbclient rename command I mean.
Comment 3 Jeremy Allison 2017-10-20 22:22:16 UTC
Created attachment 13709 [details]
git-am fix for master

Prototype fix for master. Will need back-ports for 4.7.x and 4.6.x.
Comment 4 Jeremy Allison 2017-10-20 23:26:30 UTC
Created attachment 13710 [details]
Updated git-am fix for master.

Fixes the get_cur_dir() uses in clitar.c as well.
Comment 5 Jeremy Allison 2017-10-21 00:16:19 UTC
Created attachment 13711 [details]
Updated git-am fix for master.

Sigh. Third time lucky :-). Cope correctly with pathname canonicalization for UNIX extensions paths (which should *NOT* remove \..\ components, as they can be a valid part of the filename).
Comment 6 Jeremy Allison 2017-10-23 22:44:47 UTC
Created attachment 13718 [details]
Updated git-am fix for master.

Updated with regression test. Andreas, if you're happy let me know and I'll push to master and then do the back-ports for 4.7.next, 4.6.next.
Comment 7 Andreas Schneider 2017-10-24 06:01:38 UTC
Comment on attachment 13718 [details]
Updated git-am fix for master.

Looks like you like doing the work twice. I already sent you a test last Friday via mail :-)

RB+ and pushed to master.
Comment 8 Jeremy Allison 2017-10-24 16:35:18 UTC
Oh, sorry I think I lost that email. Never mind :-).
Comment 9 Jeremy Allison 2017-10-24 23:22:50 UTC
Created attachment 13719 [details]
git-am backport for 4.7.next

Based on cherry-pick from master.
Comment 10 Jeremy Allison 2017-10-24 23:50:32 UTC
Created attachment 13720 [details]
git-am fix for 4.6.next.

Back-port from master.
Comment 11 Andreas Schneider 2017-10-26 12:23:51 UTC
Karolin, please add the patches to the relevant branches! Thanks.
Comment 12 Karolin Seeger 2017-11-01 09:38:46 UTC
(In reply to Andreas Schneider from comment #11)
Pushed to autobuild-v4-{7,6}-test.
Comment 13 Karolin Seeger 2017-11-03 08:49:21 UTC
Looks like "s3: smbclient: Test we can rename with a name containing." breaks the build as the deltree command is not available in v4-6-test. Remove the test? Backport the deltree patch?
Comment 14 Karolin Seeger 2017-11-03 08:49:49 UTC
(In reply to Karolin Seeger from comment #13)
Breaks the test, not the build, sorry! :-)
Comment 15 Karolin Seeger 2017-11-03 08:50:49 UTC
(In reply to Karolin Seeger from comment #14)
NT_STATUS_OBJECT_NAME_COLLISION making remote directory \dotdot_test\dir2
smb: \dotdot_test\> cd dir1
smb: \dotdot_test\dir1\> put  /memdisk/kseeger/a46/b3014032/samba/bin/smbclie
<t  /memdisk/kseeger/a46/b3014032/samba/bin/smbclien                         t README
putting file /memdisk/kseeger/a46/b3014032/samba/bin/smbclient as \dotdot_test\dir1\README (64471.1 kb/s) (average 64472.7 kb/s)
smb: \dotdot_test\dir1\> rename README ..\dir2\README
NT_STATUS_OBJECT_NAME_COLLISION renaming files \dotdot_test\dir1\README -> \dotdot_test\dir2\README 
smb: \dotdot_test\dir1\> cd ..
smb: \dotdot_test\> cd dir2
smb: \dotdot_test\dir2\> allinfo README
altname: README
create_time:    Thu Nov  2 10:25:11 AM 2017 CET
access_time:    Thu Nov  2 10:25:11 AM 2017 CET
write_time:     Thu Nov  2 10:25:11 AM 2017 CET
change_time:    Thu Nov  2 10:25:11 AM 2017 CET
attributes: A (20)
stream: [::$DATA], 264080 bytes
smb: \dotdot_test\dir2\> cd \
smb: \> deltree dotdot_test
deltree: command not found
smb: \> quit
Comment 16 Jeremy Allison 2017-11-03 21:09:25 UTC
I think we can just remove the test for 4.6.next. Do you want me to submit a modified patch, or do you just want to push the rb+ patch and remove the test at the end ?

Jeremy.
Comment 17 Karolin Seeger 2017-11-13 11:43:00 UTC
(In reply to Jeremy Allison from comment #16)
Pushed to v4-6-test without test.