Commit e9b33016 authored by Andrew Bartlett's avatar Andrew Bartlett Committed by Mathieu Parent

CVE-2019-3870 pysmbd: Move umask manipuations as close as possible to users

Umask manipulation was added to pysmbd with e146fe5e in 2012
and init_files_struct was split out in 747c3f1f 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=13834Signed-off-by: 's avatarAndrew Bartlett <abartlet@samba.org>

(This backport to Samba 4.9 by Andrew Bartlett is not a pure
cherry-pick due to merge conflicts)
parent 0761b15b
samba4.blackbox.provision_fileperms.provision-fileperms\(none\)
^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
......@@ -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;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment