1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
|
From 0a392c982aca2150c8350582148abd7b2af782f8 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<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.11.0
|