The Samba-Bugzilla – Attachment 15011 Details for
Bug 13851
CVE-2019-3880 [SECURITY] Save registry file outside share as unprivileged user in Samba 4.x
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for master v3.
CVE-2019-3880-master-03.patch (text/plain), 16.44 KB, created by
Jeremy Allison
on 2019-03-27 20:26:52 UTC
(
hide
)
Description:
Patch for master v3.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2019-03-27 20:26:52 UTC
Size:
16.44 KB
patch
obsolete
>From 2fdcf0a563e3547fd2e0e2c5318fe77de5bd96ce Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 21 Mar 2019 14:51:30 -0700 >Subject: [PATCH 1/2] CVE-2019-3880 s3: rpc: winreg: Remove implementations of > SaveKey/RestoreKey. > >The were not using VFS backend calls and could only work >locally, and were unsafe against symlink races and other >security issues. > >If the incoming handle is valid, return WERR_BAD_PATHNAME. > >[MS-RRP] states "The format of the file name is implementation-specific" >so ensure we don't allow this. > >As reported by Michael Hanselmann. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13851 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >--- > source3/rpc_server/winreg/srv_winreg_nt.c | 92 +---------------------- > 1 file changed, 4 insertions(+), 88 deletions(-) > >diff --git a/source3/rpc_server/winreg/srv_winreg_nt.c b/source3/rpc_server/winreg/srv_winreg_nt.c >index d9ee8d0602d..816c6bb2a12 100644 >--- a/source3/rpc_server/winreg/srv_winreg_nt.c >+++ b/source3/rpc_server/winreg/srv_winreg_nt.c >@@ -639,46 +639,6 @@ WERROR _winreg_AbortSystemShutdown(struct pipes_struct *p, > return (ret == 0) ? WERR_OK : WERR_ACCESS_DENIED; > } > >-/******************************************************************* >- ********************************************************************/ >- >-static int validate_reg_filename(TALLOC_CTX *ctx, char **pp_fname ) >-{ >- char *p = NULL; >- int num_services = lp_numservices(); >- int snum = -1; >- const char *share_path = NULL; >- char *fname = *pp_fname; >- >- /* convert to a unix path, stripping the C:\ along the way */ >- >- if (!(p = valid_share_pathname(ctx, fname))) { >- return -1; >- } >- >- /* has to exist within a valid file share */ >- >- for (snum=0; snum<num_services; snum++) { >- if (!lp_snum_ok(snum) || lp_printable(snum)) { >- continue; >- } >- >- share_path = lp_path(talloc_tos(), snum); >- >- /* make sure we have a path (e.g. [homes] ) */ >- if (strlen(share_path) == 0) { >- continue; >- } >- >- if (strncmp(share_path, p, strlen(share_path)) == 0) { >- break; >- } >- } >- >- *pp_fname = p; >- return (snum < num_services) ? snum : -1; >-} >- > /******************************************************************* > _winreg_RestoreKey > ********************************************************************/ >@@ -687,36 +647,11 @@ WERROR _winreg_RestoreKey(struct pipes_struct *p, > struct winreg_RestoreKey *r) > { > struct registry_key *regkey = find_regkey_by_hnd( p, r->in.handle ); >- char *fname = NULL; >- int snum = -1; > >- if ( !regkey ) >+ if ( !regkey ) { > return WERR_INVALID_HANDLE; >- >- if ( !r->in.filename || !r->in.filename->name ) >- return WERR_INVALID_PARAMETER; >- >- fname = talloc_strdup(p->mem_ctx, r->in.filename->name); >- if (!fname) { >- return WERR_NOT_ENOUGH_MEMORY; > } >- >- DEBUG(8,("_winreg_RestoreKey: verifying restore of key [%s] from " >- "\"%s\"\n", regkey->key->name, fname)); >- >- if ((snum = validate_reg_filename(p->mem_ctx, &fname)) == -1) >- return WERR_BAD_PATHNAME; >- >- /* user must posses SeRestorePrivilege for this this proceed */ >- >- if ( !security_token_has_privilege(p->session_info->security_token, SEC_PRIV_RESTORE)) { >- return WERR_ACCESS_DENIED; >- } >- >- DEBUG(2,("_winreg_RestoreKey: Restoring [%s] from %s in share %s\n", >- regkey->key->name, fname, lp_servicename(talloc_tos(), snum) )); >- >- return reg_restorekey(regkey, fname); >+ return WERR_BAD_PATHNAME; > } > > /******************************************************************* >@@ -727,30 +662,11 @@ WERROR _winreg_SaveKey(struct pipes_struct *p, > struct winreg_SaveKey *r) > { > struct registry_key *regkey = find_regkey_by_hnd( p, r->in.handle ); >- char *fname = NULL; >- int snum = -1; > >- if ( !regkey ) >+ if ( !regkey ) { > return WERR_INVALID_HANDLE; >- >- if ( !r->in.filename || !r->in.filename->name ) >- return WERR_INVALID_PARAMETER; >- >- fname = talloc_strdup(p->mem_ctx, r->in.filename->name); >- if (!fname) { >- return WERR_NOT_ENOUGH_MEMORY; > } >- >- DEBUG(8,("_winreg_SaveKey: verifying backup of key [%s] to \"%s\"\n", >- regkey->key->name, fname)); >- >- if ((snum = validate_reg_filename(p->mem_ctx, &fname)) == -1 ) >- return WERR_BAD_PATHNAME; >- >- DEBUG(2,("_winreg_SaveKey: Saving [%s] to %s in share %s\n", >- regkey->key->name, fname, lp_servicename(talloc_tos(), snum) )); >- >- return reg_savekey(regkey, fname); >+ return WERR_BAD_PATHNAME; > } > > /******************************************************************* >-- >2.21.0.392.gf8f6787159e-goog > > >From ea18f57d9f47b00dc63bba92394f408394877708 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 27 Mar 2019 12:51:27 -0700 >Subject: [PATCH 2/2] CVE-2019-3880 s3: rpc: winreg: Remove implementations of > SaveKey/RestoreKey. > >Remove the now unused code implementations of >registry file io. > >As reported by Michael Hanselmann. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13851 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/registry/reg_api_regf.c | 302 ---------------------- > source3/registry/reg_api_regf.h | 35 --- > source3/rpc_server/winreg/srv_winreg_nt.c | 1 - > source3/rpc_server/wscript_build | 2 +- > source3/wscript_build | 4 - > 5 files changed, 1 insertion(+), 343 deletions(-) > delete mode 100644 source3/registry/reg_api_regf.c > delete mode 100644 source3/registry/reg_api_regf.h > >diff --git a/source3/registry/reg_api_regf.c b/source3/registry/reg_api_regf.c >deleted file mode 100644 >index b83404c55e3..00000000000 >--- a/source3/registry/reg_api_regf.c >+++ /dev/null >@@ -1,302 +0,0 @@ >-/* >- * Unix SMB/CIFS implementation. >- * Virtual Windows Registry Layer >- * Copyright (C) Volker Lendecke 2006 >- * Copyright (C) Michael Adam 2007-2008 >- * >- * This program is free software; you can redistribute it and/or modify >- * it under the terms of the GNU General Public License as published by >- * the Free Software Foundation; either version 3 of the License, or >- * (at your option) any later version. >- * >- * This program is distributed in the hope that it will be useful, >- * but WITHOUT ANY WARRANTY; without even the implied warranty of >- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >- * GNU General Public License for more details. >- * >- * You should have received a copy of the GNU General Public License >- * along with this program; if not, see <http://www.gnu.org/licenses/>. >- */ >- >-#include "includes.h" >-#include "system/filesys.h" >-#include "registry.h" >-#include "reg_api_regf.h" >-#include "reg_cachehook.h" >-#include "regfio.h" >-#include "reg_util_internal.h" >-#include "reg_dispatcher.h" >- >-#undef DBGC_CLASS >-#define DBGC_CLASS DBGC_REGISTRY >- >-/******************************************************************* >- Note: topkeypat is the *full* path that this *key will be >- loaded into (including the name of the key) >- ********************************************************************/ >- >-static WERROR reg_load_tree(REGF_FILE *regfile, const char *topkeypath, >- REGF_NK_REC *key) >-{ >- REGF_NK_REC *subkey; >- struct registry_key_handle registry_key; >- struct regval_ctr *values; >- struct regsubkey_ctr *subkeys; >- int i; >- char *path = NULL; >- WERROR result = WERR_OK; >- >- /* initialize the struct registry_key_handle structure */ >- >- registry_key.ops = reghook_cache_find(topkeypath); >- if (!registry_key.ops) { >- DEBUG(0, ("reg_load_tree: Failed to assign registry_ops " >- "to [%s]\n", topkeypath)); >- return WERR_FILE_NOT_FOUND; >- } >- >- registry_key.name = talloc_strdup(regfile->mem_ctx, topkeypath); >- if (!registry_key.name) { >- DEBUG(0, ("reg_load_tree: Talloc failed for reg_key.name!\n")); >- return WERR_NOT_ENOUGH_MEMORY; >- } >- >- /* now start parsing the values and subkeys */ >- >- result = regsubkey_ctr_init(regfile->mem_ctx, &subkeys); >- W_ERROR_NOT_OK_RETURN(result); >- >- result = regval_ctr_init(subkeys, &values); >- W_ERROR_NOT_OK_RETURN(result); >- >- /* copy values into the struct regval_ctr */ >- >- for (i=0; i<key->num_values; i++) { >- regval_ctr_addvalue(values, key->values[i].valuename, >- key->values[i].type, >- key->values[i].data, >- (key->values[i].data_size & ~VK_DATA_IN_OFFSET)); >- } >- >- /* copy subkeys into the struct regsubkey_ctr */ >- >- key->subkey_index = 0; >- while ((subkey = regfio_fetch_subkey( regfile, key ))) { >- result = regsubkey_ctr_addkey(subkeys, subkey->keyname); >- if (!W_ERROR_IS_OK(result)) { >- TALLOC_FREE(subkeys); >- return result; >- } >- } >- >- /* write this key and values out */ >- >- if (!store_reg_values(®istry_key, values) >- || !store_reg_keys(®istry_key, subkeys)) >- { >- DEBUG(0,("reg_load_tree: Failed to load %s!\n", topkeypath)); >- result = WERR_REGISTRY_IO_FAILED; >- } >- >- TALLOC_FREE(subkeys); >- >- if (!W_ERROR_IS_OK(result)) { >- return result; >- } >- >- /* now continue to load each subkey registry tree */ >- >- key->subkey_index = 0; >- while ((subkey = regfio_fetch_subkey(regfile, key))) { >- path = talloc_asprintf(regfile->mem_ctx, >- "%s\\%s", >- topkeypath, >- subkey->keyname); >- if (path == NULL) { >- return WERR_NOT_ENOUGH_MEMORY; >- } >- result = reg_load_tree(regfile, path, subkey); >- if (!W_ERROR_IS_OK(result)) { >- break; >- } >- } >- >- return result; >-} >- >-/******************************************************************* >- ********************************************************************/ >- >-static WERROR restore_registry_key(struct registry_key_handle *krecord, >- const char *fname) >-{ >- REGF_FILE *regfile; >- REGF_NK_REC *rootkey; >- WERROR result; >- >- /* open the registry file....fail if the file already exists */ >- >- regfile = regfio_open(fname, (O_RDONLY), 0); >- if (regfile == NULL) { >- DEBUG(0, ("restore_registry_key: failed to open \"%s\" (%s)\n", >- fname, strerror(errno))); >- return ntstatus_to_werror(map_nt_error_from_unix(errno)); >- } >- >- /* get the rootkey from the regf file and then load the tree >- via recursive calls */ >- >- if (!(rootkey = regfio_rootkey(regfile))) { >- regfio_close(regfile); >- return WERR_NOT_REGISTRY_FILE; >- } >- >- result = reg_load_tree(regfile, krecord->name, rootkey); >- >- /* cleanup */ >- >- regfio_close(regfile); >- >- return result; >-} >- >-WERROR reg_restorekey(struct registry_key *key, const char *fname) >-{ >- return restore_registry_key(key->key, fname); >-} >- >-/******************************************************************** >-********************************************************************/ >- >-static WERROR reg_write_tree(REGF_FILE *regfile, const char *keypath, >- REGF_NK_REC *parent) >-{ >- REGF_NK_REC *key; >- struct regval_ctr *values; >- struct regsubkey_ctr *subkeys; >- int i, num_subkeys; >- char *key_tmp = NULL; >- char *keyname, *parentpath; >- char *subkeypath = NULL; >- char *subkeyname; >- struct registry_key_handle registry_key; >- WERROR result = WERR_OK; >- struct security_descriptor *sec_desc = NULL; >- >- if (!regfile) { >- return WERR_GEN_FAILURE; >- } >- >- if (!keypath) { >- return WERR_BAD_PATHNAME; >- } >- >- /* split up the registry key path */ >- >- key_tmp = talloc_strdup(regfile->mem_ctx, keypath); >- if (!key_tmp) { >- return WERR_NOT_ENOUGH_MEMORY; >- } >- if (!reg_split_key(key_tmp, &parentpath, &keyname)) { >- return WERR_BAD_PATHNAME; >- } >- >- if (!keyname) { >- keyname = parentpath; >- } >- >- /* we need a registry_key_handle object here to enumerate subkeys and values */ >- >- ZERO_STRUCT(registry_key); >- >- registry_key.name = talloc_strdup(regfile->mem_ctx, keypath); >- if (registry_key.name == NULL) { >- return WERR_NOT_ENOUGH_MEMORY; >- } >- >- registry_key.ops = reghook_cache_find(registry_key.name); >- if (registry_key.ops == NULL) { >- return WERR_FILE_NOT_FOUND; >- } >- >- /* lookup the values and subkeys */ >- >- result = regsubkey_ctr_init(regfile->mem_ctx, &subkeys); >- W_ERROR_NOT_OK_RETURN(result); >- >- result = regval_ctr_init(subkeys, &values); >- W_ERROR_NOT_OK_RETURN(result); >- >- fetch_reg_keys(®istry_key, subkeys); >- fetch_reg_values(®istry_key, values); >- >- result = regkey_get_secdesc(regfile->mem_ctx, ®istry_key, &sec_desc); >- if (!W_ERROR_IS_OK(result)) { >- goto done; >- } >- >- /* write out this key */ >- >- key = regfio_write_key(regfile, keyname, values, subkeys, sec_desc, >- parent); >- if (key == NULL) { >- result = WERR_CAN_NOT_COMPLETE; >- goto done; >- } >- >- /* write each one of the subkeys out */ >- >- num_subkeys = regsubkey_ctr_numkeys(subkeys); >- for (i=0; i<num_subkeys; i++) { >- subkeyname = regsubkey_ctr_specific_key(subkeys, i); >- subkeypath = talloc_asprintf(regfile->mem_ctx, "%s\\%s", >- keypath, subkeyname); >- if (subkeypath == NULL) { >- result = WERR_NOT_ENOUGH_MEMORY; >- goto done; >- } >- result = reg_write_tree(regfile, subkeypath, key); >- if (!W_ERROR_IS_OK(result)) >- goto done; >- } >- >- DEBUG(6, ("reg_write_tree: wrote key [%s]\n", keypath)); >- >-done: >- TALLOC_FREE(subkeys); >- TALLOC_FREE(registry_key.name); >- >- return result; >-} >- >-static WERROR backup_registry_key(struct registry_key_handle *krecord, >- const char *fname) >-{ >- REGF_FILE *regfile; >- WERROR result; >- >- /* open the registry file....fail if the file already exists */ >- >- regfile = regfio_open(fname, (O_RDWR|O_CREAT|O_EXCL), >- (S_IRUSR|S_IWUSR)); >- if (regfile == NULL) { >- DEBUG(0,("backup_registry_key: failed to open \"%s\" (%s)\n", >- fname, strerror(errno) )); >- return ntstatus_to_werror(map_nt_error_from_unix(errno)); >- } >- >- /* write the registry tree to the file */ >- >- result = reg_write_tree(regfile, krecord->name, NULL); >- >- /* cleanup */ >- >- regfio_close(regfile); >- >- return result; >-} >- >-WERROR reg_savekey(struct registry_key *key, const char *fname) >-{ >- return backup_registry_key(key->key, fname); >-} >diff --git a/source3/registry/reg_api_regf.h b/source3/registry/reg_api_regf.h >deleted file mode 100644 >index c68261fa95f..00000000000 >--- a/source3/registry/reg_api_regf.h >+++ /dev/null >@@ -1,35 +0,0 @@ >-/* >- * Unix SMB/CIFS implementation. >- * >- * Virtual Windows Registry Layer >- * >- * Copyright (C) Volker Lendecke 2006 >- * Copyright (C) Michael Adam 2007-2008 >- * >- * This program is free software; you can redistribute it and/or modify >- * it under the terms of the GNU General Public License as published by >- * the Free Software Foundation; either version 3 of the License, or >- * (at your option) any later version. >- * >- * This program is distributed in the hope that it will be useful, >- * but WITHOUT ANY WARRANTY; without even the implied warranty of >- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >- * GNU General Public License for more details. >- * >- * You should have received a copy of the GNU General Public License >- * along with this program; if not, see <http://www.gnu.org/licenses/>. >- */ >- >-/* >- * Portion of reg_api that references regfio.c code. >- * These are the savekey and restorekey calls. >- * These calls are currently only used in the WINREG rpc server. >- */ >- >-#ifndef _REG_API_REGF_H >-#define _REG_API_REGF_H >- >-WERROR reg_restorekey(struct registry_key *key, const char *fname); >-WERROR reg_savekey(struct registry_key *key, const char *fname); >- >-#endif /* _REG_API_REGF_H */ >diff --git a/source3/rpc_server/winreg/srv_winreg_nt.c b/source3/rpc_server/winreg/srv_winreg_nt.c >index 816c6bb2a12..e42f0ba203e 100644 >--- a/source3/rpc_server/winreg/srv_winreg_nt.c >+++ b/source3/rpc_server/winreg/srv_winreg_nt.c >@@ -25,7 +25,6 @@ > #include "../librpc/gen_ndr/srv_winreg.h" > #include "registry.h" > #include "registry/reg_api.h" >-#include "registry/reg_api_regf.h" > #include "registry/reg_perfcount.h" > #include "rpc_misc.h" > #include "auth.h" >diff --git a/source3/rpc_server/wscript_build b/source3/rpc_server/wscript_build >index ae75e567ace..85655d340a3 100644 >--- a/source3/rpc_server/wscript_build >+++ b/source3/rpc_server/wscript_build >@@ -131,7 +131,7 @@ bld.SAMBA3_SUBSYSTEM('RPC_SVCCTL', > bld.SAMBA3_SUBSYSTEM('RPC_WINREG', > source='''winreg/srv_winreg_nt.c > ../../librpc/gen_ndr/srv_winreg.c''', >- deps='REG_FULL REGFIO REG_API_REGF NDR_PERFCOUNT') >+ deps='REG_FULL REGFIO NDR_PERFCOUNT') > > bld.SAMBA3_SUBSYSTEM('RPC_WKSSVC', > source='''wkssvc/srv_wkssvc_nt.c >diff --git a/source3/wscript_build b/source3/wscript_build >index 317896eee46..a8c0fac513a 100644 >--- a/source3/wscript_build >+++ b/source3/wscript_build >@@ -209,10 +209,6 @@ bld.SAMBA_BINARY('test_registry_regfio', > local_include=False, > install=False) > >-bld.SAMBA3_SUBSYSTEM('REG_API_REGF', >- source='registry/reg_api_regf.c', >- deps='samba-util') >- > # Do not link against this use 'smbconf' > bld.SAMBA3_SUBSYSTEM('SMBREGISTRY', > source=''' >-- >2.21.0.392.gf8f6787159e-goog >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=15011">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
abartlet
:
review+
Actions:
View
Attachments on
bug 13851
:
14974
|
14975
|
14996
|
14997
|
14998
|
14999
|
15000
|
15001
|
15010
| 15011 |
15012
|
15028
|
15030