Bug 12885 - CVE-2017-2619 breaks accessing previous versions of directories with snapshots in subdirectories of the share
Summary: CVE-2017-2619 breaks accessing previous versions of directories with snapshot...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 regression (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-07 11:22 UTC by Ralph Böhme
Modified: 2017-07-25 09:28 UTC (History)
2 users (show)

See Also:


Attachments
Patch for 4.5 cherry-picked from master (4.60 KB, patch)
2017-07-10 09:33 UTC, Ralph Böhme
jra: review+
Details
Patch for 4.6 cherry-picked from master (4.60 KB, patch)
2017-07-10 09:34 UTC, Ralph Böhme
jra: review+
Details
Patch for 4.7 cherry-picked from master (4.66 KB, patch)
2017-07-10 09:34 UTC, Ralph Böhme
jra: review+
Details
Patch for 4.4 cherry-picked from master, ommitting test (2.59 KB, patch)
2017-07-14 08:38 UTC, Ralph Böhme
slow: review? (metze)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Böhme 2017-07-07 11:22:43 UTC
With shadow:snapdirseverywhere=true and a snapshot directory that

* is a subdirectory of a share

* and that contains a snapshot directory

we fail the symlink check in the new function non_widelink_open() because parent_dirname() cuts off the subdirectory name leaving only the @GMT stanza which is then interpreted by the called functions as being relative to the parent directory which it isn't.

The simplest fix as far as I can see is to leverage the fact that (given the system defines O_DIRECTORY) we know when we're called for a directory, so we can just directly chdir() into the path passed by the caller.

The subsequent security check done in check_reduced_name() should continue to work with this change.
Comment 1 Ralph Böhme 2017-07-07 11:23:01 UTC
Btw, have patch and test, need bugnumber...
Comment 2 Ralph Böhme 2017-07-10 09:33:49 UTC
Created attachment 13350 [details]
Patch for 4.5 cherry-picked from master
Comment 3 Ralph Böhme 2017-07-10 09:34:11 UTC
Created attachment 13351 [details]
Patch for 4.6 cherry-picked from master
Comment 4 Ralph Böhme 2017-07-10 09:34:34 UTC
Created attachment 13352 [details]
Patch for 4.7 cherry-picked from master
Comment 5 Jeremy Allison 2017-07-10 18:39:09 UTC
Re-assigning to Karolin for inclusion in 4.7.0, 4.6.next, 4.5.next.
Comment 6 Stefan Metzmacher 2017-07-13 05:04:35 UTC
Pushed to autobuild-v4-{5,6,7}-test
Comment 7 Stefan Metzmacher 2017-07-14 07:49:17 UTC
(In reply to Stefan Metzmacher from comment #6)

Pushed to v4-{5,6,7}.

Should we also push that to v4-4, so that the next security release also
picks it up? As this fixes a regression introduced by a security release.
Comment 8 Ralph Böhme 2017-07-14 08:38:32 UTC
Created attachment 13381 [details]
Patch for 4.4 cherry-picked from master, ommitting test
Comment 9 Jeremy Allison 2017-07-14 15:56:32 UTC
(In reply to Stefan Metzmacher from comment #7)

Yes, I think fixing in 4.4.x is appropriate here.
Comment 10 Karolin Seeger 2017-07-23 20:04:33 UTC
(In reply to Jeremy Allison from comment #9)
Pushed to autobuild-v4-4-test.
Comment 11 Karolin Seeger 2017-07-25 09:28:47 UTC
(In reply to Karolin Seeger from comment #10)
Pushed to all branches.
Closing out bug report.

Thanks!