170 lines
5.4 KiB
Diff
170 lines
5.4 KiB
Diff
From c99f2ab22cc93b5194a3477c6a241600fa0f6758 Mon Sep 17 00:00:00 2001
|
|
From: Andrew Bartlett <abartlet@samba.org>
|
|
Date: Thu, 14 Mar 2019 18:20:06 +1300
|
|
Subject: [PATCH 4/5] CVE-2019-3870 pysmbd: Move umask manipuations as close as
|
|
possible to users
|
|
|
|
Umask manipulation was added to pysmbd with e146fe5ef96c1522175a8e81db15d1e8879e5652 in 2012
|
|
and init_files_struct was split out in 747c3f1fb379bb68cc7479501b85741493c05812 in 2018 for
|
|
Samba 4.9. (It was added to assist the smbd.create_file() routine used in the backup and
|
|
restore tools, which needed to write files with full metadata).
|
|
|
|
This in turn avoids leaving init_files_struct() without resetting the umask to
|
|
the original, saved, value.
|
|
|
|
Per umask(2) this is required before open() and mkdir() system calls (along
|
|
side other file-like things such as those for Unix domain socks and FIFOs etc).
|
|
|
|
Therefore for safety and clarify the additional 'belt and braces' umask
|
|
manipuations elsewhere are removed.
|
|
|
|
mkdir() will be protected by a umask() bracket, for correctness, in the next patch.
|
|
|
|
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13834
|
|
|
|
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
|
|
|
|
(This backport to Samba 4.9 by Andrew Bartlett is not a pure
|
|
cherry-pick due to merge conflicts)
|
|
---
|
|
selftest/knownfail.d/provision_fileperms | 1 -
|
|
selftest/knownfail.d/umask-leak | 3 ---
|
|
source3/smbd/pysmbd.c | 34 ++++++++++----------------------
|
|
3 files changed, 10 insertions(+), 28 deletions(-)
|
|
delete mode 100644 selftest/knownfail.d/provision_fileperms
|
|
delete mode 100644 selftest/knownfail.d/umask-leak
|
|
|
|
diff --git a/selftest/knownfail.d/provision_fileperms b/selftest/knownfail.d/provision_fileperms
|
|
deleted file mode 100644
|
|
index 88b1585fd19..00000000000
|
|
--- a/selftest/knownfail.d/provision_fileperms
|
|
+++ /dev/null
|
|
@@ -1 +0,0 @@
|
|
-samba4.blackbox.provision_fileperms.provision-fileperms\(none\)
|
|
diff --git a/selftest/knownfail.d/umask-leak b/selftest/knownfail.d/umask-leak
|
|
deleted file mode 100644
|
|
index 5580beb4b68..00000000000
|
|
--- a/selftest/knownfail.d/umask-leak
|
|
+++ /dev/null
|
|
@@ -1,3 +0,0 @@
|
|
-^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_smbd_create_file
|
|
-^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_backup_online
|
|
-^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_backup_offline
|
|
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
|
|
index 1431925efd0..179a1ee2943 100644
|
|
--- a/source3/smbd/pysmbd.c
|
|
+++ b/source3/smbd/pysmbd.c
|
|
@@ -85,27 +85,19 @@ static int set_sys_acl_conn(const char *fname,
|
|
{
|
|
int ret;
|
|
struct smb_filename *smb_fname = NULL;
|
|
- mode_t saved_umask;
|
|
|
|
TALLOC_CTX *frame = talloc_stackframe();
|
|
|
|
- /* we want total control over the permissions on created files,
|
|
- so set our umask to 0 */
|
|
- saved_umask = umask(0);
|
|
-
|
|
smb_fname = synthetic_smb_fname_split(frame,
|
|
fname,
|
|
lp_posix_pathnames());
|
|
if (smb_fname == NULL) {
|
|
TALLOC_FREE(frame);
|
|
- umask(saved_umask);
|
|
return -1;
|
|
}
|
|
|
|
ret = SMB_VFS_SYS_ACL_SET_FILE( conn, smb_fname, acltype, theacl);
|
|
|
|
- umask(saved_umask);
|
|
-
|
|
TALLOC_FREE(frame);
|
|
return ret;
|
|
}
|
|
@@ -132,22 +124,26 @@ static NTSTATUS init_files_struct(TALLOC_CTX *mem_ctx,
|
|
}
|
|
fsp->conn = conn;
|
|
|
|
- /* we want total control over the permissions on created files,
|
|
- so set our umask to 0 */
|
|
- saved_umask = umask(0);
|
|
-
|
|
smb_fname = synthetic_smb_fname_split(fsp,
|
|
fname,
|
|
lp_posix_pathnames());
|
|
if (smb_fname == NULL) {
|
|
- umask(saved_umask);
|
|
return NT_STATUS_NO_MEMORY;
|
|
}
|
|
|
|
fsp->fsp_name = smb_fname;
|
|
+
|
|
+ /*
|
|
+ * we want total control over the permissions on created files,
|
|
+ * so set our umask to 0 (this matters if flags contains O_CREAT)
|
|
+ */
|
|
+ saved_umask = umask(0);
|
|
+
|
|
fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, 00644);
|
|
+
|
|
+ umask(saved_umask);
|
|
+
|
|
if (fsp->fh->fd == -1) {
|
|
- umask(saved_umask);
|
|
return NT_STATUS_INVALID_PARAMETER;
|
|
}
|
|
|
|
@@ -157,7 +153,6 @@ static NTSTATUS init_files_struct(TALLOC_CTX *mem_ctx,
|
|
DEBUG(0,("Error doing fstat on open file %s (%s)\n",
|
|
smb_fname_str_dbg(smb_fname),
|
|
strerror(errno) ));
|
|
- umask(saved_umask);
|
|
return map_nt_error_from_unix(errno);
|
|
}
|
|
|
|
@@ -444,7 +439,6 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
|
|
char *fname, *service = NULL;
|
|
int uid, gid;
|
|
TALLOC_CTX *frame;
|
|
- mode_t saved_umask;
|
|
struct smb_filename *smb_fname = NULL;
|
|
|
|
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "sii|z",
|
|
@@ -460,10 +454,6 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
|
|
return NULL;
|
|
}
|
|
|
|
- /* we want total control over the permissions on created files,
|
|
- so set our umask to 0 */
|
|
- saved_umask = umask(0);
|
|
-
|
|
smb_fname = synthetic_smb_fname(talloc_tos(),
|
|
fname,
|
|
NULL,
|
|
@@ -471,7 +461,6 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
|
|
lp_posix_pathnames() ?
|
|
SMB_FILENAME_POSIX_PATH : 0);
|
|
if (smb_fname == NULL) {
|
|
- umask(saved_umask);
|
|
TALLOC_FREE(frame);
|
|
errno = ENOMEM;
|
|
return PyErr_SetFromErrno(PyExc_OSError);
|
|
@@ -479,14 +468,11 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
|
|
|
|
ret = SMB_VFS_CHOWN(conn, smb_fname, uid, gid);
|
|
if (ret != 0) {
|
|
- umask(saved_umask);
|
|
TALLOC_FREE(frame);
|
|
errno = ret;
|
|
return PyErr_SetFromErrno(PyExc_OSError);
|
|
}
|
|
|
|
- umask(saved_umask);
|
|
-
|
|
TALLOC_FREE(frame);
|
|
|
|
Py_RETURN_NONE;
|
|
--
|
|
2.11.0
|