[PATCH] s3/smbd: use stat from smb_fname if valid in refuse_symlink()

[Ralph Böhme]

On Sun, Sep 11, 2016 at 10:43:11AM -0700, Jeremy Allison wrote:
> On Sun, Sep 11, 2016 at 03:53:16PM +0200, Ralph Böhme wrote:
> > On Sun, Sep 11, 2016 at 01:42:54PM +0200, Ralph Böhme wrote:
> > > On Sun, Sep 11, 2016 at 01:09:43PM +0200, Ralph Böhme wrote:
> > > > I think we can safely save one stat call in refuse_symlink(). Please
> > > > review carefully & push if ok.
> > > > 
> > > > refuse_symlink() was added as part of CVE-2015-7560, bug 11648 in
> > > > commit b551cd83ef74340adaf88629a9ee9fa5c5215ec6 taking a char *path
> > > > and an fsp, so obviously a stat optimisation could only be done for
> > > > the case a valid fsp was passed.
> > > > 
> > > > A later change in 13dae2b46ed9a53b7eeed4ce125478b5bbb3e2b5 changed the
> > > > function signature to take a struct smb_filename * instead of a char *.
> > > 
> > > it fails the POSIX-SYMLINK-EA test. :/ Glad we have it! Looking
> > > into it.
> > 
> > got it, looks like we have a bug in call_trans2qfilepathinfo() for
> > requests with POSIX paths: we're calling SMB_VFS_STAT() instead of
> > lstat on the smb_filename, even though the request is in POSIX
> > context. Is this correct? I don't think so.
> > 
> > Not sure how bad this is, but changing the check that switches between
> > stat and lstat to do an lstat for UNIX info levels *and* if the
> > request if flagged as using POSIX pathnames fixes my refuse_symlink()
> > optimisation.
> > 
> > Please review & push if ok. Passes smbtorture_s3 tests, currently
> > running a complete autobuild.
> 
> LGTM - we will need a bugid for this to backport to 4.5.1.

that's just for patch 1/2, right?

> Good catch !

I'm soooo glad that your test caught this!

Cheerio!
-slow