From b8a0cfe737e7078860ce8e78395d4b790cfe645e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Mon, 27 Mar 2017 10:46:47 -0700
Subject: [PATCH 1/6] 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@samba.org>
Reviewed-by: Uri Simchoni <uri@samba.org>
(cherry picked from commit ae17bebd250bdde5614b2ac17e53512f19fe9b68)
---
 source3/smbd/vfs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 6c56964628a..8a4de58446d 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -1008,8 +1008,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;
 			}
 
-- 
2.12.0


From 2c6de8584779e413f1e6ff9c933f9281693bfbc0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Mon, 27 Mar 2017 11:48:25 -0700
Subject: [PATCH 2/6] 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@samba.org>
Reviewed-by: Uri Simchoni <uri@samba.org>

Back-ported from commit 782172a9bef0040981d20e49519b13dd744df6a0
---
 selftest/target/Samba3.pm                 |  7 +++
 source3/script/tests/test_smbclient_s3.sh | 73 +++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 01a1c470af0..7765b9efbb2 100644
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -479,6 +479,9 @@ sub provision($$$$$$)
 	my $msdfs_deeppath="$msdfs_shrdir/deeppath";
 	push(@dirs,$msdfs_deeppath);
 
+	my $nosymlinks_shrdir="$shrdir/nosymlinks";
+	push(@dirs,$nosymlinks_shrdir);
+
 	# this gets autocreated by winbindd
 	my $wbsockdir="$prefix_abs/winbindd";
 	my $wbsockprivdir="$lockdir/winbindd_privileged";
@@ -695,6 +698,10 @@ sub provision($$$$$$)
 	copy = print1
 [print\$]
 	copy = tmp
+[nosymlinks]
+       copy = tmp
+       path = $nosymlinks_shrdir
+       follow symlinks = no
 	";
 	close(CONF);
 
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 772802f77b1..57ef87e4949 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -401,6 +401,75 @@ done
 
 LOGDIR=$(mktemp -d ${PREFIX}/${LOGDIR_PREFIX}_XXXXXX)
 
+# 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"
+
+    rm -f $slink_target
+    rm -f $slink_name
+    rm -rf $mkdir_target
+
+    touch $slink_target
+    ln -s $slink_target $slink_name
+
+# 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 != 0 ] ; then
+       echo "$out"
+       echo "failed accessing nosymlinks with error $ret"
+       false
+       return
+    fi
+
+    echo "$out" | grep 'NT_STATUS_ACCESS_DENIED'
+    ret=$?
+    if [ $ret != 0 ] ; then
+       echo "$out"
+       echo "failed - should get NT_STATUS_ACCESS_DENIED getting \\nosymlinks\\source"
+       false
+    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 != 0 ] ; then
+       echo "$out"
+       echo "failed accessing nosymlinks with error $ret"
+       false
+       return
+    fi
+
+    echo "$out" | grep 'NT_STATUS'
+    ret=$?
+    if [ $ret == 0 ] ; then
+	echo "$out"
+	echo "failed - NT_STATUS_XXXX doing mkdir a; mkdir a\\b on \\nosymlinks"
+	false
+    fi
+}
 
 testit "smbclient -L $SERVER_IP" $SMBCLIENT -L $SERVER_IP -N -p 139 || failed=`expr $failed + 1`
 testit "smbclient -L $SERVER -I $SERVER_IP" $SMBCLIENT -L $SERVER -I $SERVER_IP -N -p 139 -c quit || failed=`expr $failed + 1`
@@ -445,6 +514,10 @@ testit "ccache access works for smbclient" \
     test_ccache_access || \
     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`
-- 
2.12.0


From 17865cf188f42850f18f46514643a5b3a43e5707 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Mon, 27 Mar 2017 22:07:50 -0700
Subject: [PATCH 3/6] 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@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit 037297a1c50e90a0092e3b94f472623f41ccc015)
---
 source3/script/tests/test_smbclient_s3.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 57ef87e4949..bd5714fca6e 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -428,7 +428,7 @@ EOF
     ret=$?
     rm -f $tmpfile
 
-    if [ $ret != 0 ] ; then
+    if [ $ret -ne 0 ] ; then
        echo "$out"
        echo "failed accessing nosymlinks with error $ret"
        false
@@ -437,10 +437,11 @@ EOF
 
     echo "$out" | grep 'NT_STATUS_ACCESS_DENIED'
     ret=$?
-    if [ $ret != 0 ] ; then
+    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.
@@ -455,7 +456,7 @@ EOF
     ret=$?
     rm -f $tmpfile
 
-    if [ $ret != 0 ] ; then
+    if [ $ret -ne 0 ] ; then
        echo "$out"
        echo "failed accessing nosymlinks with error $ret"
        false
@@ -464,7 +465,7 @@ EOF
 
     echo "$out" | grep 'NT_STATUS'
     ret=$?
-    if [ $ret == 0 ] ; then
+    if [ $ret -eq 0 ] ; then
 	echo "$out"
 	echo "failed - NT_STATUS_XXXX doing mkdir a; mkdir a\\b on \\nosymlinks"
 	false
-- 
2.12.0


From f66f71d4d57985f4cb1f5775d03a29445a10e5cc Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Mon, 27 Mar 2017 17:04:58 -0700
Subject: [PATCH 4/6] 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@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit 83e30cb48859b412b76572b6a3ba84d8fde167af)
---
 source3/smbd/filename.c |  2 +-
 source3/smbd/open.c     |  2 +-
 source3/smbd/proto.h    |  4 +++-
 source3/smbd/vfs.c      | 10 +++++++++-
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 8ef0c0a9579..bfe3b1d47b0 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1033,7 +1033,7 @@ NTSTATUS check_name(connection_struct *conn, const char *name)
 	}
 
 	if (!lp_widelinks(SNUM(conn)) || !lp_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 65ca14ec8b8..22f103d3758 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -388,7 +388,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 8d78a7178af..cdebbb19d77 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1178,7 +1178,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);
 int vfs_stat_smb_fname(struct connection_struct *conn, const char *fname,
 		       SMB_STRUCT_STAT *psbuf);
 int vfs_lstat_smb_fname(struct connection_struct *conn, const char *fname,
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 8a4de58446d..0b62275db79 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -894,9 +894,17 @@ char *vfs_GetWd(TALLOC_CTX *ctx, 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;
 	bool allow_symlinks = true;
-- 
2.12.0


From 727d27372f3a42d3d6724b60705b8b4e5c2d9db7 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Mon, 27 Mar 2017 17:09:38 -0700
Subject: [PATCH 5/6] 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@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit e182a4d39e86c9694e255efdf6ee2ea3ccb9af4a)
---
 source3/smbd/vfs.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 0b62275db79..09ad66175f5 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -907,6 +907,7 @@ NTSTATUS check_reduced_name(connection_struct *conn,
 				const char *fname)
 {
 	char *resolved_name = NULL;
+	char *new_fname = NULL;
 	bool allow_symlinks = true;
 	bool allow_widelinks = false;
 
@@ -1034,6 +1035,26 @@ NTSTATUS check_reduced_name(connection_struct *conn,
 			}
 
 			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\n",
@@ -1049,6 +1070,7 @@ NTSTATUS check_reduced_name(connection_struct *conn,
 	DEBUG(3,("check_reduced_name: %s reduced to %s\n", fname,
 		 resolved_name));
 	SAFE_FREE(resolved_name);
+	TALLOC_FREE(new_fname);
 	return NT_STATUS_OK;
 }
 
-- 
2.12.0


From 9b573af39f3d4995464e30771fa06e0709b5e57b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Mon, 27 Mar 2017 22:10:29 -0700
Subject: [PATCH 6/6] s3: Test for CVE-2017-2619 regression with "follow
 symlinks = no" - part 2
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add tests for regular access.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Tue Mar 28 17:05:27 CEST 2017 on sn-devel-144

(cherry picked from commit 4e734fcd1bf82c08aa303ce44e9735acccffcf06)
---
 source3/script/tests/test_smbclient_s3.sh | 37 +++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index bd5714fca6e..885766f6c16 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -408,14 +408,22 @@ test_nosymlinks()
     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
@@ -470,6 +478,35 @@ EOF
 	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
 }
 
 testit "smbclient -L $SERVER_IP" $SMBCLIENT -L $SERVER_IP -N -p 139 || failed=`expr $failed + 1`
-- 
2.12.0