[SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

The branch, master has been updated
       via  2f0cfe82907 s3: smbd: Fix uninitialized memory read in process_symlink_open() when used with vfs_shadow_copy2().
      from  a4c0666f6bc docs-xml: Update pdbedit manpage for new cmdline opition parser

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 2f0cfe82907516ecf23cc385d41b8d29ed6b8c96
Author: Jeremy Allison <jra at samba.org>
Date:   Wed May 26 22:41:53 2021 -0700

    s3: smbd: Fix uninitialized memory read in process_symlink_open() when used with vfs_shadow_copy2().
    
    Valgrind trace follows.
    
    ==3627798== Invalid read of size 1
    ==3627798==    at 0x483FF46: strlen (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==3627798==    by 0x55DE412: strdup (strdup.c:41)
    ==3627798==    by 0x4F4657E: smb_xstrdup (util.c:660)
    ==3627798==    by 0x4C62C2E: vfs_ChDir (vfs.c:988)
    ==3627798==    by 0x4C4A51C: process_symlink_open (open.c:656)
    ==3627798==    by 0x4C4ADE7: non_widelink_open (open.c:862)
    ==3627798==    by 0x4C4AFB7: fd_openat (open.c:918)
    ==3627798==    by 0x4BBE895: openat_pathref_fsp (files.c:506)
    ==3627798==    by 0x4C48A00: filename_convert_internal (filename.c:2027)
    ==3627798==    by 0x4C48B77: filename_convert (filename.c:2067)
    ==3627798==    by 0x4C32408: call_trans2qfilepathinfo (trans2.c:6173)
    ==3627798==    by 0x4C3C5DA: handle_trans2 (trans2.c:10143)
    ==3627798==  Address 0xda8bc90 is 96 bytes inside a block of size 217 free'd
    ==3627798==    at 0x483DA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==3627798==    by 0x4FCA3C9: _tc_free_internal (talloc.c:1222)
    ==3627798==    by 0x4FCA481: _talloc_free_internal (talloc.c:1248)
    ==3627798==    by 0x4FCB825: _talloc_free (talloc.c:1792)
    ==3627798==    by 0xDB248DD: store_cwd_data (vfs_shadow_copy2.c:1473)
    ==3627798==    by 0xDB24BEF: shadow_copy2_chdir (vfs_shadow_copy2.c:1542)
    ==3627798==    by 0x4C662A4: smb_vfs_call_chdir (vfs.c:2257)
    ==3627798==    by 0x4C62B48: vfs_ChDir (vfs.c:940)
    ==3627798==    by 0x4C4A51C: process_symlink_open (open.c:656)
    ==3627798==    by 0x4C4ADE7: non_widelink_open (open.c:862)
    ==3627798==    by 0x4C4AFB7: fd_openat (open.c:918)
    ==3627798==    by 0x4BBE895: openat_pathref_fsp (files.c:506)
    ==3627798==  Block was alloc'd at
    ==3627798==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==3627798==    by 0x4FC9365: __talloc_with_prefix (talloc.c:783)
    ==3627798==    by 0x4FC94FF: __talloc (talloc.c:825)
    ==3627798==    by 0x4FCCFDC: __talloc_strlendup (talloc.c:2454)
    ==3627798==    by 0x4FCD096: talloc_strdup (talloc.c:2470)
    ==3627798==    by 0xDB24977: store_cwd_data (vfs_shadow_copy2.c:1476)
    ==3627798==    by 0xDB24BEF: shadow_copy2_chdir (vfs_shadow_copy2.c:1542)
    ==3627798==    by 0x4C662A4: smb_vfs_call_chdir (vfs.c:2257)
    ==3627798==    by 0x4C62B48: vfs_ChDir (vfs.c:940)
    ==3627798==    by 0x4C4A92D: non_widelink_open (open.c:755)
    ==3627798==    by 0x4C4AFB7: fd_openat (open.c:918)
    ==3627798==    by 0x4BBE895: openat_pathref_fsp (files.c:506)
    ==3627798==
    
    Even though SMB_VFS_CONNECTPATH() returns a const char,
    vfs_shadow_copy2() can free and reallocate this whilst
    in use inside process_symlink_open().
    
    Take a copy to make sure we don't reference free'd memory.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14721
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Böhme <slow at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Thu May 27 17:25:43 UTC 2021 on sn-devel-184

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

Summary of changes:
 source3/smbd/open.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 4a5d57cfb7b..919ccf4cab8 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -532,7 +532,7 @@ static NTSTATUS process_symlink_open(const struct files_struct *dirfsp,
 {
 	struct connection_struct *conn = dirfsp->conn;
 	const char *conn_rootdir = NULL;
-	struct smb_filename conn_rootdir_fname;
+	struct smb_filename conn_rootdir_fname = { 0 };
 	char *link_target = NULL;
 	int link_len = -1;
 	struct smb_filename *oldwd_fname = NULL;
@@ -547,9 +547,15 @@ static NTSTATUS process_symlink_open(const struct files_struct *dirfsp,
 	if (conn_rootdir == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
-	conn_rootdir_fname = (struct smb_filename) {
-		.base_name = discard_const_p(char, conn_rootdir),
-	};
+	/*
+	 * With shadow_copy2 conn_rootdir can be talloc_freed
+	 * whilst we use it in this function. We must take a copy.
+	 */
+	conn_rootdir_fname.base_name = talloc_strdup(talloc_tos(),
+						     conn_rootdir);
+	if (conn_rootdir_fname.base_name == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
 
 	/*
 	 * Ensure we don't get stuck in a symlink loop.
@@ -669,6 +675,7 @@ static NTSTATUS process_symlink_open(const struct files_struct *dirfsp,
 
 	TALLOC_FREE(resolved_fname);
 	TALLOC_FREE(link_target);
+	TALLOC_FREE(conn_rootdir_fname.base_name);
 	if (oldwd_fname != NULL) {
 		int ret = vfs_ChDir(conn, oldwd_fname);
 		if (ret == -1) {


-- 
Samba Shared Repository