From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 11 Jul 2019 15:02:15 +0000 (+0200)
Subject: CVE-2019-10197: smbd: split change_to_user_impersonate() out of change_to_user_internal()
X-Git-Tag: tevent-0.10.1~33
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=efd4832c2cfc4092bafb93dd4caf32d04488919f;p=thirdparty%2Fsamba.git

CVE-2019-10197: smbd: split change_to_user_impersonate() out of change_to_user_internal()

This makes sure we always call chdir_current_service() even
when we still impersonated the user. Which is important
in order to run the SMB* request within the correct working directory
and only if the user has permissions to enter that directory.

It makes sure we always update conn->lastused_count
in chdir_current_service() for each request.

Note that vfs_ChDir() (called from chdir_current_service())
maintains its own cache and avoids calling SMB_VFS_CHDIR()
if possible.

It means we still avoid syscalls if we get a multiple requests
for the same session/tcon tuple.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(master): Tue Sep  3 09:27:22 UTC 2019 on sn-devel-184
---

diff --git a/selftest/knownfail.d/CVE-2019-10197 b/selftest/knownfail.d/CVE-2019-10197
deleted file mode 100644
index f7056bbf3ad..00000000000
--- a/selftest/knownfail.d/CVE-2019-10197
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.smbclient_s3.*.noperm.share.regression
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index ac2f645e6ca..5425449d42c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -306,9 +306,9 @@ static void print_impersonation_info(connection_struct *conn)
  stack, but modify the current_user entries.
 ****************************************************************************/
 
-static bool change_to_user_internal(connection_struct *conn,
-				    const struct auth_session_info *session_info,
-				    uint64_t vuid)
+static bool change_to_user_impersonate(connection_struct *conn,
+				       const struct auth_session_info *session_info,
+				       uint64_t vuid)
 {
 	int snum;
 	gid_t gid;
@@ -321,7 +321,6 @@ static bool change_to_user_internal(connection_struct *conn,
 
 	if ((current_user.conn == conn) &&
 	    (current_user.vuid == vuid) &&
-	    (current_user.need_chdir == conn->tcon_done) &&
 	    (current_user.ut.uid == session_info->unix_token->uid))
 	{
 		DBG_INFO("Skipping user change - already user\n");
@@ -426,6 +425,20 @@ static bool change_to_user_internal(connection_struct *conn,
 
 	current_user.conn = conn;
 	current_user.vuid = vuid;
+	return true;
+}
+
+static bool change_to_user_internal(connection_struct *conn,
+				    const struct auth_session_info *session_info,
+				    uint64_t vuid)
+{
+	bool ok;
+
+	ok = change_to_user_impersonate(conn, session_info, vuid);
+	if (!ok) {
+		return false;
+	}
+
 	current_user.need_chdir = conn->tcon_done;
 	current_user.done_chdir = false;