From a803d2524b8c06e2c360db0c686a212ac49f7321 Mon Sep 17 00:00:00 2001

From: Jeremy Allison <jra@samba.org>

Date: Thu, 21 Mar 2019 14:51:30 -0700

Subject: [PATCH] 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

@@ -640,46 +640,6 @@ WERROR _winreg_AbortSystemShutdown(struct pipes_struct *p,

 }

 /*******************************************************************

- ********************************************************************/

-

-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

-		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.11.0