[PATCH] Fix bug 13565 - vfs_audit log does not show full path names

Jeremy Allison jra at samba.org
Fri Aug 24 23:09:04 UTC 2018


FYI, The security fix for CVE-2017-2619 (symlink race fix)
changed the pathnames logged by the vfs_full_audit module
to be relative instead of absolute.

This fixes that.

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

Please review and push if happy, and I'll back-port to
relevent 4.9.x branches.

Cheers,

	Jeremy.
-------------- next part --------------
From c6daf27d48e74b3cc1ddcf33a4f81ddacbc58efc Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 24 Aug 2018 13:17:24 -0700
Subject: [PATCH 1/2] s3: VFS: vfs_full_audit: Add $cwd arg to
 smb_fname_str_do_log().

Not yet used.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_full_audit.c | 36 ++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/source3/modules/vfs_full_audit.c b/source3/modules/vfs_full_audit.c
index 0fbf9ecafe3..b7929605bfa 100644
--- a/source3/modules/vfs_full_audit.c
+++ b/source3/modules/vfs_full_audit.c
@@ -658,7 +658,8 @@ static void do_log(vfs_op_type op, bool success, vfs_handle_struct *handle,
 /**
  * Return a string using the do_log_ctx()
  */
-static const char *smb_fname_str_do_log(const struct smb_filename *smb_fname)
+static const char *smb_fname_str_do_log(const struct smb_filename *cwd,
+				const struct smb_filename *smb_fname)
 {
 	char *fname = NULL;
 	NTSTATUS status;
@@ -678,7 +679,7 @@ static const char *smb_fname_str_do_log(const struct smb_filename *smb_fname)
  */
 static const char *fsp_str_do_log(const struct files_struct *fsp)
 {
-	return smb_fname_str_do_log(fsp->fsp_name);
+	return smb_fname_str_do_log(fsp->conn->cwd_fname, fsp->fsp_name);
 }
 
 /* Implementation of vfs_ops.  Pass everything on to the default
@@ -1017,7 +1018,7 @@ static int smb_full_audit_open(vfs_handle_struct *handle,
 
 	do_log(SMB_VFS_OP_OPEN, (result >= 0), handle, "%s|%s",
 	       ((flags & O_WRONLY) || (flags & O_RDWR))?"w":"r",
-	       smb_fname_str_do_log(smb_fname));
+	       smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return result;
 }
@@ -1091,7 +1092,8 @@ static NTSTATUS smb_full_audit_create_file(vfs_handle_struct *handle,
 	do_log(SMB_VFS_OP_CREATE_FILE, (NT_STATUS_IS_OK(result)), handle,
 	       "0x%x|%s|%s|%s", access_mask,
 	       create_options & FILE_DIRECTORY_FILE ? "dir" : "file",
-	       str_create_disposition, smb_fname_str_do_log(smb_fname));
+	       str_create_disposition,
+		smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return result;
 }
@@ -1330,8 +1332,8 @@ static int smb_full_audit_rename(vfs_handle_struct *handle,
 	result = SMB_VFS_NEXT_RENAME(handle, smb_fname_src, smb_fname_dst);
 
 	do_log(SMB_VFS_OP_RENAME, (result >= 0), handle, "%s|%s",
-	       smb_fname_str_do_log(smb_fname_src),
-	       smb_fname_str_do_log(smb_fname_dst));
+	       smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname_src),
+	       smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname_dst));
 
 	return result;    
 }
@@ -1413,7 +1415,7 @@ static int smb_full_audit_stat(vfs_handle_struct *handle,
 	result = SMB_VFS_NEXT_STAT(handle, smb_fname);
 
 	do_log(SMB_VFS_OP_STAT, (result >= 0), handle, "%s",
-	       smb_fname_str_do_log(smb_fname));
+	       smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return result;    
 }
@@ -1439,7 +1441,7 @@ static int smb_full_audit_lstat(vfs_handle_struct *handle,
 	result = SMB_VFS_NEXT_LSTAT(handle, smb_fname);
 
 	do_log(SMB_VFS_OP_LSTAT, (result >= 0), handle, "%s",
-	       smb_fname_str_do_log(smb_fname));
+	       smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return result;    
 }
@@ -1465,7 +1467,7 @@ static int smb_full_audit_unlink(vfs_handle_struct *handle,
 	result = SMB_VFS_NEXT_UNLINK(handle, smb_fname);
 
 	do_log(SMB_VFS_OP_UNLINK, (result >= 0), handle, "%s",
-	       smb_fname_str_do_log(smb_fname));
+	       smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return result;
 }
@@ -1576,7 +1578,7 @@ static int smb_full_audit_ntimes(vfs_handle_struct *handle,
 	result = SMB_VFS_NEXT_NTIMES(handle, smb_fname, ft);
 
 	do_log(SMB_VFS_OP_NTIMES, (result >= 0), handle, "%s",
-	       smb_fname_str_do_log(smb_fname));
+	       smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return result;
 }
@@ -2013,7 +2015,8 @@ static NTSTATUS smb_full_audit_get_compression(vfs_handle_struct *handle,
 
 	do_log(SMB_VFS_OP_GET_COMPRESSION, NT_STATUS_IS_OK(result), handle,
 	       "%s",
-	       (fsp ? fsp_str_do_log(fsp) : smb_fname_str_do_log(smb_fname)));
+	       (fsp ? fsp_str_do_log(fsp) :
+		smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname)));
 
 	return result;
 }
@@ -2044,7 +2047,7 @@ static NTSTATUS smb_full_audit_readdir_attr(struct vfs_handle_struct *handle,
 	status = SMB_VFS_NEXT_READDIR_ATTR(handle, fname, mem_ctx, pattr_data);
 
 	do_log(SMB_VFS_OP_READDIR_ATTR, NT_STATUS_IS_OK(status), handle, "%s",
-	       smb_fname_str_do_log(fname));
+	       smb_fname_str_do_log(handle->conn->cwd_fname, fname));
 
 	return status;
 }
@@ -2064,7 +2067,7 @@ static NTSTATUS smb_full_audit_get_dos_attributes(
 		NT_STATUS_IS_OK(status),
 		handle,
 		"%s",
-		smb_fname_str_do_log(smb_fname));
+		smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return status;
 }
@@ -2226,7 +2229,7 @@ static NTSTATUS smb_full_audit_set_dos_attributes(
 		NT_STATUS_IS_OK(status),
 		handle,
 		"%s",
-		smb_fname_str_do_log(smb_fname));
+		smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return status;
 }
@@ -2279,7 +2282,7 @@ static NTSTATUS smb_full_audit_get_nt_acl(vfs_handle_struct *handle,
 					 mem_ctx, ppdesc);
 
 	do_log(SMB_VFS_OP_GET_NT_ACL, NT_STATUS_IS_OK(result), handle,
-	       "%s", smb_fname_str_do_log(smb_fname));
+	       "%s", smb_fname_str_do_log(handle->conn->cwd_fname, smb_fname));
 
 	return result;
 }
@@ -2325,7 +2328,8 @@ static NTSTATUS smb_full_audit_audit_file(struct vfs_handle_struct *handle,
 					access_denied);
 
 	do_log(SMB_VFS_OP_AUDIT_FILE, NT_STATUS_IS_OK(result), handle,
-			"%s", smb_fname_str_do_log(file));
+			"%s",
+			smb_fname_str_do_log(handle->conn->cwd_fname, file));
 
 	return result;
 }
-- 
2.19.0.rc0.228.g281dcd1b4d0-goog


From 21caa1116a25b56afa849910b02e1b5ea37e7620 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 24 Aug 2018 13:37:27 -0700
Subject: [PATCH 2/2] s3: VFS: vfs_full_audit: Ensure smb_fname_str_do_log()
 only returns absolute pathnames.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_full_audit.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/source3/modules/vfs_full_audit.c b/source3/modules/vfs_full_audit.c
index b7929605bfa..e7ca89f9fbe 100644
--- a/source3/modules/vfs_full_audit.c
+++ b/source3/modules/vfs_full_audit.c
@@ -667,6 +667,32 @@ static const char *smb_fname_str_do_log(const struct smb_filename *cwd,
 	if (smb_fname == NULL) {
 		return "";
 	}
+
+	if (smb_fname->base_name[0] != '/') {
+		char *abs_name = NULL;
+		struct smb_filename *fname_copy = cp_smb_filename(
+							do_log_ctx(),
+							smb_fname);
+		if (fname_copy == NULL) {
+			return "";
+		}
+
+		if (!ISDOT(smb_fname->base_name)) {
+			abs_name = talloc_asprintf(do_log_ctx(),
+					"%s/%s",
+					cwd->base_name,
+					smb_fname->base_name);
+		} else {
+			abs_name = talloc_strdup(do_log_ctx(),
+					cwd->base_name);
+		}
+		if (abs_name == NULL) {
+			return "";
+		}
+		fname_copy->base_name = abs_name;
+		smb_fname = fname_copy;
+	}
+
 	status = get_full_smb_filename(do_log_ctx(), smb_fname, &fname);
 	if (!NT_STATUS_IS_OK(status)) {
 		return "";
-- 
2.19.0.rc0.228.g281dcd1b4d0-goog



More information about the samba-technical mailing list