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
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
|
From b66f369e343e3b7c4920512d6eac427f23ae3b89 Mon Sep 17 00:00:00 2001
From: Honza Horak <hhorak@redhat.com>
Date: Thu, 8 Dec 2016 10:31:58 +0100
Subject: [PATCH] Bug#24388746: PRIVILEGE ESCALATION AND RACE CONDITION USING
CREATE TABLE
MySQL 5.1 upstream backport of:
https://github.com/mysql/mysql-server/commit/4e5473862e6852b0f3802b0cd0c6fa10b5253291
During REPAIR TABLE of a MyISAM table, a temporary data file (.TMD)
is created. When repair finishes, this file is renamed to the original
.MYD file. The problem was that during this rename, we copied the
stats from the old file to the new file with chmod/chown. If a user
managed to replace the temporary file before chmod/chown was executed,
it was possible to get an arbitrary file with the privileges of the
mysql user.
This patch fixes the problem by not copying stats from the old
file to the new file. This is not needed as the new file was
created with the correct stats. This fix only changes server
behavior - external utilities such as myisamchk still does
chmod/chown.
No test case provided since the problem involves synchronization
with file system operations.
---
include/my_sys.h | 1 +
include/myisam.h | 9 +++++----
mysys/my_redel.c | 10 ++++++++--
storage/myisam/ha_myisam.cc | 24 ++++++++++++++++++++----
storage/myisam/mi_check.c | 39 ++++++++++++++++++++++++++++-----------
storage/myisam/myisamchk.c | 14 +++++++++-----
6 files changed, 71 insertions(+), 26 deletions(-)
diff --git a/include/my_sys.h b/include/my_sys.h
index a2742e4..2620168 100644
--- a/include/my_sys.h
+++ b/include/my_sys.h
@@ -72,6 +72,7 @@ extern int NEAR my_errno; /* Last error in mysys */
#define MY_RESOLVE_LINK 128 /* my_realpath(); Only resolve links */
#define MY_HOLD_ORIGINAL_MODES 128 /* my_copy() holds to file modes */
#define MY_REDEL_MAKE_BACKUP 256
+#define MY_REDEL_NO_COPY_STAT 512 /* my_redel() doesn't call my_copystat() */
#define MY_SEEK_NOT_DONE 32 /* my_lock may have to do a seek */
#define MY_DONT_WAIT 64 /* my_lock() don't wait if can't lock */
#define MY_ZEROFILL 32 /* my_malloc(), fill array with zero */
diff --git a/include/myisam.h b/include/myisam.h
index 92e6699..5ae177c 100644
--- a/include/myisam.h
+++ b/include/myisam.h
@@ -498,12 +498,13 @@ int chk_size(MI_CHECK *param, MI_INFO *info);
int chk_key(MI_CHECK *param, MI_INFO *info);
int chk_data_link(MI_CHECK *param, MI_INFO *info,int extend);
int mi_repair(MI_CHECK *param, register MI_INFO *info,
- char * name, int rep_quick);
-int mi_sort_index(MI_CHECK *param, register MI_INFO *info, char * name);
+ char * name, int rep_quick, my_bool no_copy_stat);
+int mi_sort_index(MI_CHECK *param, register MI_INFO *info, char * name,
+ my_bool no_copy_stat);
int mi_repair_by_sort(MI_CHECK *param, register MI_INFO *info,
- const char * name, int rep_quick);
+ const char * name, int rep_quick, my_bool no_copy_stat);
int mi_repair_parallel(MI_CHECK *param, register MI_INFO *info,
- const char * name, int rep_quick);
+ const char * name, int rep_quick, my_bool no_copy_stat);
int change_to_newfile(const char * filename, const char * old_ext,
const char * new_ext, uint raid_chunks,
myf myflags);
diff --git a/mysys/my_redel.c b/mysys/my_redel.c
index f8d43d2..0859696 100644
--- a/mysys/my_redel.c
+++ b/mysys/my_redel.c
@@ -37,6 +37,9 @@ struct utimbuf {
if MY_REDEL_MAKE_COPY is given, then the orginal file
is renamed to org_name-'current_time'.BAK
+
+ if MY_REDEL_NO_COPY_STAT is given, stats are not copied
+ from org_name to tmp_name.
*/
#define REDEL_EXT ".BAK"
@@ -48,8 +51,11 @@ int my_redel(const char *org_name, const char *tmp_name, myf MyFlags)
DBUG_PRINT("my",("org_name: '%s' tmp_name: '%s' MyFlags: %d",
org_name,tmp_name,MyFlags));
- if (my_copystat(org_name,tmp_name,MyFlags) < 0)
- goto end;
+ if (!(MyFlags & MY_REDEL_NO_COPY_STAT))
+ {
+ if (my_copystat(org_name,tmp_name,MyFlags) < 0)
+ goto end;
+ }
if (MyFlags & MY_REDEL_MAKE_BACKUP)
{
char name_buff[FN_REFLEN+20];
diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc
index 173dc35..9a561f5 100644
--- a/storage/myisam/ha_myisam.cc
+++ b/storage/myisam/ha_myisam.cc
@@ -1159,24 +1159,36 @@ int ha_myisam::repair(THD *thd, MI_CHECK ¶m, bool do_optimize)
/* TODO: respect myisam_repair_threads variable */
my_snprintf(buf, 40, "Repair with %d threads", my_count_bits(key_map));
thd_proc_info(thd, buf);
+ /*
+ * The new file is created with the right stats, so we can skip
+ * copying file stats from old to new.
+ */
error = mi_repair_parallel(¶m, file, fixed_name,
- param.testflag & T_QUICK);
+ param.testflag & T_QUICK, TRUE);
thd_proc_info(thd, "Repair done"); // to reset proc_info, as
// it was pointing to local buffer
}
else
{
thd_proc_info(thd, "Repair by sorting");
+ /*
+ * The new file is created with the right stats, so we can skip
+ * copying file stats from old to new.
+ */
error = mi_repair_by_sort(¶m, file, fixed_name,
- param.testflag & T_QUICK);
+ param.testflag & T_QUICK, TRUE);
}
}
else
{
thd_proc_info(thd, "Repair with keycache");
param.testflag &= ~T_REP_BY_SORT;
+ /*
+ * The new file is created with the right stats, so we can skip
+ * copying file stats from old to new.
+ */
error= mi_repair(¶m, file, fixed_name,
- param.testflag & T_QUICK);
+ param.testflag & T_QUICK, TRUE);
}
#ifdef HAVE_MMAP
if (remap)
@@ -1192,7 +1204,11 @@ int ha_myisam::repair(THD *thd, MI_CHECK ¶m, bool do_optimize)
{
optimize_done=1;
thd_proc_info(thd, "Sorting index");
- error=mi_sort_index(¶m,file,fixed_name);
+ /*
+ * The new file is created with the right stats, so we can skip
+ * copying file stats from old to new.
+ */
+ error=mi_sort_index(¶m,file,fixed_name, TRUE);
}
if (!statistics_done && (local_testflag & T_STATISTICS))
{
diff --git a/storage/myisam/mi_check.c b/storage/myisam/mi_check.c
index 698c1f3..b071061 100644
--- a/storage/myisam/mi_check.c
+++ b/storage/myisam/mi_check.c
@@ -1520,7 +1520,7 @@ static int mi_drop_all_indexes(MI_CHECK *param, MI_INFO *info, my_bool force)
/* Save new datafile-name in temp_filename */
int mi_repair(MI_CHECK *param, register MI_INFO *info,
- char * name, int rep_quick)
+ char * name, int rep_quick, my_bool no_copy_stat)
{
int error,got_error;
ha_rows start_records,new_header_length;
@@ -1736,12 +1736,16 @@ err:
/* Replace the actual file with the temporary file */
if (new_file >= 0)
{
+ myf flags= 0;
+ if (param->testflag & T_BACKUP_DATA)
+ flags |= MY_REDEL_MAKE_BACKUP;
+ if (no_copy_stat)
+ flags |= MY_REDEL_NO_COPY_STAT;
my_close(new_file,MYF(0));
info->dfile=new_file= -1;
if (change_to_newfile(share->data_file_name,MI_NAME_DEXT,
DATA_TMP_EXT, share->base.raid_chunks,
- (param->testflag & T_BACKUP_DATA ?
- MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
+ flags) ||
mi_open_datafile(info,share,name,-1))
got_error=1;
@@ -1931,7 +1935,8 @@ int flush_blocks(MI_CHECK *param, KEY_CACHE *key_cache, File file)
/* Sort index for more efficent reads */
-int mi_sort_index(MI_CHECK *param, register MI_INFO *info, char * name)
+int mi_sort_index(MI_CHECK *param, register MI_INFO *info, char * name,
+ my_bool no_copy_stat)
{
reg2 uint key;
reg1 MI_KEYDEF *keyinfo;
@@ -2000,7 +2005,7 @@ int mi_sort_index(MI_CHECK *param, register MI_INFO *info, char * name)
share->kfile = -1;
VOID(my_close(new_file,MYF(MY_WME)));
if (change_to_newfile(share->index_file_name,MI_NAME_IEXT,INDEX_TMP_EXT,0,
- MYF(0)) ||
+ no_copy_stat ? MYF(MY_REDEL_NO_COPY_STAT) : MYF(0)) ||
mi_open_keyfile(share))
goto err2;
info->lock_type= F_UNLCK; /* Force mi_readinfo to lock */
@@ -2213,6 +2218,8 @@ err:
info MyISAM handler to repair
name Name of table (for warnings)
rep_quick set to <> 0 if we should not change data file
+ no_copy_stat Don't copy file stats from old to new file,
+ assume that new file was created with correct stats
RESULT
0 ok
@@ -2220,7 +2227,7 @@ err:
*/
int mi_repair_by_sort(MI_CHECK *param, register MI_INFO *info,
- const char * name, int rep_quick)
+ const char * name, int rep_quick, my_bool no_copy_stat)
{
int got_error;
uint i;
@@ -2549,12 +2556,16 @@ err:
/* Replace the actual file with the temporary file */
if (new_file >= 0)
{
+ myf flags= 0;
+ if (param->testflag & T_BACKUP_DATA)
+ flags |= MY_REDEL_MAKE_BACKUP;
+ if (no_copy_stat)
+ flags |= MY_REDEL_NO_COPY_STAT;
my_close(new_file,MYF(0));
info->dfile=new_file= -1;
if (change_to_newfile(share->data_file_name,MI_NAME_DEXT,
DATA_TMP_EXT, share->base.raid_chunks,
- (param->testflag & T_BACKUP_DATA ?
- MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
+ flags) ||
mi_open_datafile(info,share,name,-1))
got_error=1;
}
@@ -2604,6 +2615,8 @@ err:
info MyISAM handler to repair
name Name of table (for warnings)
rep_quick set to <> 0 if we should not change data file
+ no_copy_stat Don't copy file stats from old to new file,
+ assume that new file was created with correct stats
DESCRIPTION
Same as mi_repair_by_sort but do it multithreaded
@@ -2638,7 +2651,7 @@ err:
*/
int mi_repair_parallel(MI_CHECK *param, register MI_INFO *info,
- const char * name, int rep_quick)
+ const char * name, int rep_quick, my_bool no_copy_stat)
{
#ifndef THREAD
return mi_repair_by_sort(param, info, name, rep_quick);
@@ -3086,12 +3099,16 @@ err:
/* Replace the actual file with the temporary file */
if (new_file >= 0)
{
+ myf flags= 0;
+ if (param->testflag & T_BACKUP_DATA)
+ flags |= MY_REDEL_MAKE_BACKUP;
+ if (no_copy_stat)
+ flags |= MY_REDEL_NO_COPY_STAT;
my_close(new_file,MYF(0));
info->dfile=new_file= -1;
if (change_to_newfile(share->data_file_name,MI_NAME_DEXT,
DATA_TMP_EXT, share->base.raid_chunks,
- (param->testflag & T_BACKUP_DATA ?
- MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
+ flags) ||
mi_open_datafile(info,share,name,-1))
got_error=1;
}
diff --git a/storage/myisam/myisamchk.c b/storage/myisam/myisamchk.c
index 282a02a..bd14c34 100644
--- a/storage/myisam/myisamchk.c
+++ b/storage/myisam/myisamchk.c
@@ -1020,14 +1020,18 @@ static int myisamchk(MI_CHECK *param, char * filename)
info->s->state.key_map,
param->force_sort))
{
+ /*
+ The new file might not be created with the right stats depending
+ on how myisamchk is run, so we must copy file stats from old to new.
+ */
if (param->testflag & T_REP_BY_SORT)
- error=mi_repair_by_sort(param,info,filename,rep_quick);
+ error=mi_repair_by_sort(param,info,filename,rep_quick,FALSE);
else
- error=mi_repair_parallel(param,info,filename,rep_quick);
+ error=mi_repair_parallel(param,info,filename,rep_quick,FALSE);
state_updated=1;
}
else if (param->testflag & T_REP_ANY)
- error=mi_repair(param, info,filename,rep_quick);
+ error=mi_repair(param, info,filename,rep_quick,FALSE);
}
if (!error && param->testflag & T_SORT_RECORDS)
{
@@ -1069,12 +1073,12 @@ static int myisamchk(MI_CHECK *param, char * filename)
{
if (param->verbose)
puts("Table had a compressed index; We must now recreate the index");
- error=mi_repair_by_sort(param,info,filename,1);
+ error=mi_repair_by_sort(param,info,filename,1,FALSE);
}
}
}
if (!error && param->testflag & T_SORT_INDEX)
- error=mi_sort_index(param,info,filename);
+ error=mi_sort_index(param,info,filename,FALSE);
if (!error)
share->state.changed&= ~(STATE_CHANGED | STATE_CRASHED |
STATE_CRASHED_ON_REPAIR);
--
2.7.4
|