[SCM] Samba Shared Repository - branch v4-5-stable updated

Karolin Seeger kseeger at samba.org
Fri Mar 31 06:55:33 UTC 2017


The branch, v4-5-stable has been updated
       via  964d1fc VERSION: Disable GIT_SNAPSHOTS for the 4.5.8 release.
       via  46032b9 WHATSNEW: Add release notes for Samba 4.5.8.
       via  e013dc8 s3: Test for CVE-2017-2619 regression with "follow symlinks = no" - part 2
       via  8dfe455 s3: smbd: Fix "follow symlink = no" regression part 2.
       via  95c190d s3: smbd: Fix "follow symlink = no" regression part 2.
       via  99378b7 s3: Fixup test for CVE-2017-2619 regression with "follow symlinks = no"
       via  29dd6b1 s3: Test for CVE-2017-2619 regression with "follow symlinks = no".
       via  42fd34a s3: smbd: Fix incorrect logic exposed by fix for the security bug 12496 (CVE-2017-2619).
       via  4e28a8f VERSION: Re-enable GIT_SNAPSHOTS.
       via  bf889af VERSION: Up to Samba 4.5.8.
      from  3da28b8 VERSION: Disable GIT_SNAPSHOTS for the 4.5.7 release.

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-5-stable


- Log -----------------------------------------------------------------
commit 964d1fc423e5d464d9f0cbda7cbbd7b4d1f6b41c
Author: Karolin Seeger <kseeger at samba.org>
Date:   Fri Mar 31 08:28:44 2017 +0200

    VERSION: Disable GIT_SNAPSHOTS for the 4.5.8 release.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit 46032b93c557eb2c5e7a081c54560f175dcc1d8f
Author: Karolin Seeger <kseeger at samba.org>
Date:   Fri Mar 31 08:27:31 2017 +0200

    WHATSNEW: Add release notes for Samba 4.5.8.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit e013dc86145827d36cba810a41cfbf44e55d6113
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Mar 27 22:10:29 2017 -0700

    s3: Test for CVE-2017-2619 regression with "follow symlinks = no" - part 2
    
    Add tests for regular access.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Tue Mar 28 17:05:27 CEST 2017 on sn-devel-144
    
    (cherry picked from commit 4e734fcd1bf82c08aa303ce44e9735acccffcf06)

commit 8dfe45570eac697d23088037c9d48d4b52966b95
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Mar 27 17:09:38 2017 -0700

    s3: smbd: Fix "follow symlink = no" regression part 2.
    
    Use the cwd_name parameter to reconstruct the original
    client name for symlink testing.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit e182a4d39e86c9694e255efdf6ee2ea3ccb9af4a)

commit 95c190dc7e024c2ee603c681252fc29bb9315b06
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Mar 27 17:04:58 2017 -0700

    s3: smbd: Fix "follow symlink = no" regression part 2.
    
    Add an extra paramter to cwd_name to check_reduced_name().
    
    If cwd_name == NULL then fname is a client given path relative
    to the root path of the share.
    
    If cwd_name != NULL then fname is a client given path relative
    to cwd_name. cwd_name is relative to the root path of the share.
    
    Not yet used, logic added in the next commit.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 83e30cb48859b412b76572b6a3ba84d8fde167af)

commit 99378b758d8b0e8d250f024e9dfb378fda2c96aa
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Mar 27 22:07:50 2017 -0700

    s3: Fixup test for CVE-2017-2619 regression with "follow symlinks = no"
    
    Use correct bash operators (not string operators).
    Add missing "return".
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 037297a1c50e90a0092e3b94f472623f41ccc015)

commit 29dd6b1316af22236c20719ae676648a346eff3e
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Mar 27 11:48:25 2017 -0700

    s3: Test for CVE-2017-2619 regression with "follow symlinks = no".
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Uri Simchoni <uri at samba.org>
    
    Back-ported from commit 782172a9bef0040981d20e49519b13dd744df6a0

commit 42fd34aa5ee1ad60b4593a1cddeaba4a5dbc2ea6
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Mar 27 10:46:47 2017 -0700

    s3: smbd: Fix incorrect logic exposed by fix for the security bug 12496 (CVE-2017-2619).
    
    In a UNIX filesystem, the names "." and ".." by definition can *never*
    be symlinks - they are already reserved names.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Uri Simchoni <uri at samba.org>
    (cherry picked from commit ae17bebd250bdde5614b2ac17e53512f19fe9b68)

commit 4e28a8f7642bc44ccebf88e2b6aeeac879b41a0e
Author: Karolin Seeger <kseeger at samba.org>
Date:   Fri Mar 31 08:24:44 2017 +0200

    VERSION: Re-enable GIT_SNAPSHOTS.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit bf889af0c8289b2a96b7a5393d150e09f8a091e9
Author: Karolin Seeger <kseeger at samba.org>
Date:   Thu Mar 23 10:20:48 2017 +0100

    VERSION: Up to Samba 4.5.8.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>
    (cherry picked from commit d537977721ee10d198ced5fd6ab141fe0636e28e)

-----------------------------------------------------------------------

Summary of changes:
 VERSION                                   |   2 +-
 WHATSNEW.txt                              |  45 +++++++++++-
 selftest/target/Samba3.pm                 |   7 ++
 source3/script/tests/test_smbclient_s3.sh | 111 ++++++++++++++++++++++++++++++
 source3/smbd/filename.c                   |   2 +-
 source3/smbd/open.c                       |   2 +-
 source3/smbd/proto.h                      |   4 +-
 source3/smbd/vfs.c                        |  40 ++++++++++-
 8 files changed, 204 insertions(+), 9 deletions(-)


Changeset truncated at 500 lines:

diff --git a/VERSION b/VERSION
index 25685f5..a40efff 100644
--- a/VERSION
+++ b/VERSION
@@ -25,7 +25,7 @@
 ########################################################
 SAMBA_VERSION_MAJOR=4
 SAMBA_VERSION_MINOR=5
-SAMBA_VERSION_RELEASE=7
+SAMBA_VERSION_RELEASE=8
 
 ########################################################
 # If a official release has a serious bug              #
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 591fbc6..5f37176 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,4 +1,45 @@
                    =============================
+                   Release Notes for Samba 4.5.8
+                           March 31, 2017
+                   =============================
+
+
+This is a bug fix release to address a regression introduced by the security
+fixes for CVE-2017-2619 (Symlink race allows access outside share definition).
+Please see https://bugzilla.samba.org/show_bug.cgi?id=12721 for details.
+
+
+Changes since 4.5.7:
+--------------------
+
+o  Jeremy Allison <jra at samba.org>
+   * BUG 12721: Fix regression with "follow symlinks = no".
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical IRC channel on irc.freenode.net.
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored.  All bug reports should
+be filed under the Samba 4.1 and newer product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
+
+                   =============================
                    Release Notes for Samba 4.5.7
                            March 23, 2017
                    =============================
@@ -66,8 +107,8 @@ database (https://bugzilla.samba.org/).
 ======================================================================
 
 
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
+
 
                    =============================
                    Release Notes for Samba 4.5.6
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 938d195..1424a53 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1207,6 +1207,9 @@ sub provision($$$$$$$$)
 	my $shadow_shrdir="$shadow_basedir/share";
 	push(@dirs,$shadow_shrdir);
 
+	my $nosymlinks_shrdir="$shrdir/nosymlinks";
+	push(@dirs,$nosymlinks_shrdir);
+
 	# this gets autocreated by winbindd
 	my $wbsockdir="$prefix_abs/winbindd";
 	my $wbsockprivdir="$lockdir/winbindd_privileged";
@@ -1820,6 +1823,10 @@ sub provision($$$$$$$$)
 	copy = tmp
 	acl_xattr:ignore system acls = yes
 	acl_xattr:default acl style = posix
+[nosymlinks]
+	copy = tmp
+	path = $nosymlinks_shrdir
+	follow symlinks = no
 [acl_xattr_ign_sysacl_windows]
 	copy = tmp
 	acl_xattr:ignore system acls = yes
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 22849bd..9bff883 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -1096,6 +1096,113 @@ EOF
     fi
 }
 
+# Test follow symlinks can't access symlinks
+test_nosymlinks()
+{
+# Setup test dirs.
+    slink_name="$LOCAL_PATH/nosymlinks/source"
+    slink_target="$LOCAL_PATH/nosymlinks/target"
+    mkdir_target="$LOCAL_PATH/nosymlinks/a"
+    dir1="$LOCAL_PATH/nosymlinks/foo"
+    dir2="$LOCAL_PATH/nosymlinks/foo/bar"
+    get_target="$LOCAL_PATH/nosymlinks/foo/bar/testfile"
+
+    rm -f $slink_target
+    rm -f $slink_name
+    rm -rf $mkdir_target
+    rm -rf $dir1
+
+    touch $slink_target
+    ln -s $slink_target $slink_name
+
+    mkdir $dir1
+    mkdir $dir2
+    touch $get_target
+
+# Getting a file through a symlink name should fail.
+    tmpfile=$PREFIX/smbclient_interactive_prompt_commands
+    cat > $tmpfile <<EOF
+get source
+quit
+EOF
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/nosymlinks -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    rm -f $tmpfile
+
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed accessing nosymlinks with error $ret"
+       false
+       return
+    fi
+
+    echo "$out" | grep 'NT_STATUS_ACCESS_DENIED'
+    ret=$?
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed - should get NT_STATUS_ACCESS_DENIED getting \\nosymlinks\\source"
+       false
+       return
+    fi
+
+# But we should be able to create and delete directories.
+    cat > $tmpfile <<EOF
+mkdir a
+mkdir a\\b
+quit
+EOF
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/nosymlinks -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    rm -f $tmpfile
+
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed accessing nosymlinks with error $ret"
+       false
+       return
+    fi
+
+    echo "$out" | grep 'NT_STATUS'
+    ret=$?
+    if [ $ret -eq 0 ] ; then
+	echo "$out"
+	echo "failed - NT_STATUS_XXXX doing mkdir a; mkdir a\\b on \\nosymlinks"
+	false
+    fi
+
+# Ensure regular file/directory access also works.
+    cat > $tmpfile <<EOF
+cd foo\\bar
+ls
+get testfile -
+quit
+EOF
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/nosymlinks -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    rm -f $tmpfile
+
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed accessing nosymlinks with error $ret"
+       false
+       return
+    fi
+
+    echo "$out" | grep 'NT_STATUS'
+    ret=$?
+    if [ $ret -eq 0 ] ; then
+       echo "$out"
+       echo "failed - NT_STATUS_XXXX doing cd foo\\bar; get testfile on \\nosymlinks"
+       false
+       return
+    fi
+}
 
 LOGDIR_PREFIX=test_smbclient_s3
 
@@ -1196,6 +1303,10 @@ testit "streams_depot can delete correctly" \
     test_streams_depot_delete || \
     failed=`expr $failed + 1`
 
+testit "follow symlinks = no" \
+    test_nosymlinks || \
+    failed=`expr $failed + 1`
+
 testit "rm -rf $LOGDIR" \
     rm -rf $LOGDIR || \
     failed=`expr $failed + 1`
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index efe52a0..2d85e8d 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1242,7 +1242,7 @@ NTSTATUS check_name(connection_struct *conn, const char *name)
 	}
 
 	if (!lp_widelinks(SNUM(conn)) || !lp_follow_symlinks(SNUM(conn))) {
-		status = check_reduced_name(conn,name);
+		status = check_reduced_name(conn, NULL, name);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(5,("check_name: name %s failed with %s\n",name,
 						nt_errstr(status)));
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 006be91..5fc3049 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -549,7 +549,7 @@ static int non_widelink_open(struct connection_struct *conn,
 	}
 
 	/* Ensure the relative path is below the share. */
-	status = check_reduced_name(conn, final_component);
+	status = check_reduced_name(conn, parent_dir, final_component);
 	if (!NT_STATUS_IS_OK(status)) {
 		saved_errno = map_errno_from_nt_status(status);
 		goto out;
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 50ede9d..61bd33a 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1227,7 +1227,9 @@ const char *vfs_readdirname(connection_struct *conn, void *p,
 			    SMB_STRUCT_STAT *sbuf, char **talloced);
 int vfs_ChDir(connection_struct *conn, const char *path);
 char *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn);
-NTSTATUS check_reduced_name(connection_struct *conn, const char *fname);
+NTSTATUS check_reduced_name(connection_struct *conn,
+			const char *cwd_name,
+			const char *fname);
 NTSTATUS check_reduced_name_with_privilege(connection_struct *conn,
 			const char *fname,
 			struct smb_request *smbreq);
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 45562ee..d24b42b 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -1179,11 +1179,20 @@ NTSTATUS check_reduced_name_with_privilege(connection_struct *conn,
 /*******************************************************************
  Reduce a file name, removing .. elements and checking that
  it is below dir in the heirachy. This uses realpath.
+
+ If cwd_name == NULL then fname is a client given path relative
+ to the root path of the share.
+
+ If cwd_name != NULL then fname is a client given path relative
+ to cwd_name. cwd_name is relative to the root path of the share.
 ********************************************************************/
 
-NTSTATUS check_reduced_name(connection_struct *conn, const char *fname)
+NTSTATUS check_reduced_name(connection_struct *conn,
+				const char *cwd_name,
+				const char *fname)
 {
 	char *resolved_name = NULL;
+	char *new_fname = NULL;
 	bool allow_symlinks = true;
 	bool allow_widelinks = false;
 
@@ -1307,8 +1316,11 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname)
 			/* fname can't have changed in resolved_path. */
 			const char *p = &resolved_name[rootdir_len];
 
-			/* *p can be '\0' if fname was "." */
-			if (*p == '\0' && ISDOT(fname)) {
+			/*
+			 * UNIX filesystem semantics, names consisting
+			 * only of "." or ".." CANNOT be symlinks.
+			 */
+			if (ISDOT(fname) || ISDOTDOT(fname)) {
 				goto out;
 			}
 
@@ -1322,11 +1334,32 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname)
 			}
 
 			p++;
+
+			/*
+			 * If cwd_name is present and not ".",
+			 * then fname is relative to that, not
+			 * the root of the share. Make sure the
+			 * path we check is the one the client
+			 * sent (cwd_name+fname).
+			 */
+			if (cwd_name != NULL && !ISDOT(cwd_name)) {
+				new_fname = talloc_asprintf(talloc_tos(),
+							"%s/%s",
+							cwd_name,
+							fname);
+				if (new_fname == NULL) {
+					SAFE_FREE(resolved_name);
+					return NT_STATUS_NO_MEMORY;
+				}
+				fname = new_fname;
+			}
+
 			if (strcmp(fname, p)!=0) {
 				DEBUG(2, ("check_reduced_name: Bad access "
 					"attempt: %s is a symlink to %s\n",
 					  fname, p));
 				SAFE_FREE(resolved_name);
+				TALLOC_FREE(new_fname);
 				return NT_STATUS_ACCESS_DENIED;
 			}
 		}
@@ -1336,6 +1369,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname)
 
 	DBG_INFO("%s reduced to %s\n", fname, resolved_name);
 	SAFE_FREE(resolved_name);
+	TALLOC_FREE(new_fname);
 	return NT_STATUS_OK;
 }
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list