[PATCH] s3/smbd: use stat from smb_fname if valid in refuse_symlink()
Ralph Böhme
slow at samba.org
Sun Sep 11 13:53:16 UTC 2016
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.
Cheerio!
-slow
-------------- next part --------------
From 399d53a63322af12c63136a90066d49f137ef97a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 11 Sep 2016 15:35:37 +0200
Subject: [PATCH 1/2] s3/smbd: in call_trans2qfilepathinfo call lstat when
dealing with posix pathnames
This might be an info level SMB_INFO_QUERY_ALL_EAS which is not covered
by INFO_LEVEL_IS_UNIX(). If smb_fname is a symlink we would then stat it
in POSIX context.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/trans2.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 1775316..a01bb81 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -5767,7 +5767,8 @@ static void call_trans2qfilepathinfo(connection_struct *conn,
}
if (info_level == SMB_QUERY_FILE_UNIX_BASIC ||
info_level == SMB_QUERY_FILE_UNIX_INFO2 ||
- info_level == SMB_QUERY_FILE_UNIX_LINK) {
+ info_level == SMB_QUERY_FILE_UNIX_LINK ||
+ req->posix_pathnames) {
ucf_flags |= UCF_UNIX_NAME_LOOKUP;
}
}
@@ -5831,7 +5832,7 @@ static void call_trans2qfilepathinfo(connection_struct *conn,
return;
}
- if (INFO_LEVEL_IS_UNIX(info_level)) {
+ if (INFO_LEVEL_IS_UNIX(info_level) || req->posix_pathnames) {
/* Always do lstat for UNIX calls. */
if (SMB_VFS_LSTAT(conn, smb_fname_base) != 0) {
DEBUG(3,("call_trans2qfilepathinfo: "
@@ -5877,7 +5878,7 @@ static void call_trans2qfilepathinfo(connection_struct *conn,
}
}
- if (INFO_LEVEL_IS_UNIX(info_level)) {
+ if (INFO_LEVEL_IS_UNIX(info_level) || req->posix_pathnames) {
/* Always do lstat for UNIX calls. */
if (SMB_VFS_LSTAT(conn, smb_fname)) {
DEBUG(3,("call_trans2qfilepathinfo: "
--
2.7.4
From bf752b5c85a132d29c256821e9671a3b74e99bd5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 10 Sep 2016 14:43:07 +0200
Subject: [PATCH 2/2] s3/smbd: use stat from smb_fname if valid in
refuse_symlink()
Now that refuse_symlink() gets passed in a smb_fname and not just a char
buffer, we can try to reuse its stat info and save one stat call here.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/trans2.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index a01bb81..6999b2d 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -55,7 +55,7 @@ static char *store_file_unix_basic_info2(connection_struct *conn,
const SMB_STRUCT_STAT *psbuf);
/****************************************************************************
- Check if an open file handle or pathname is a symlink.
+ Check if an open file handle or smb_fname is a symlink.
****************************************************************************/
static NTSTATUS refuse_symlink(connection_struct *conn,
@@ -68,6 +68,10 @@ static NTSTATUS refuse_symlink(connection_struct *conn,
if (fsp) {
pst = &fsp->fsp_name->st;
} else {
+ pst = &smb_fname->st;
+ }
+
+ if (!VALID_STAT(*pst)) {
int ret = vfs_stat_smb_basename(conn,
smb_fname,
&sbuf);
@@ -76,6 +80,7 @@ static NTSTATUS refuse_symlink(connection_struct *conn,
}
pst = &sbuf;
}
+
if (S_ISLNK(pst->st_ex_mode)) {
return NT_STATUS_ACCESS_DENIED;
}
--
2.7.4
More information about the samba-technical
mailing list