Skip to content

Commit a926eb8

Browse files
committed
Ensure that all temp files made during pg_upgrade are non-world-readable.
pg_upgrade has always attempted to ensure that the transient dump files it creates are inaccessible except to the owner. However, refactoring in commit 76a7650 broke that for the file containing "pg_dumpall -g" output; since then, that file was protected according to the process's default umask. Since that file may contain role passwords (hopefully encrypted, but passwords nonetheless), this is a particularly unfortunate oversight. Prudent users of pg_upgrade on multiuser systems would probably run it under a umask tight enough that the issue is moot, but perhaps some users are depending only on pg_upgrade's umask changes to protect their data. To fix this in a future-proof way, let's just tighten the umask at process start. There are no files pg_upgrade needs to write at a weaker security level; and if there were, transiently relaxing the umask around where they're created would be a safer approach. Report and patch by Tom Lane; the idea for the fix is due to Noah Misch. Back-patch to all supported branches. Security: CVE-2018-1053
1 parent 3492a0a commit a926eb8

File tree

4 files changed

+7
-26
lines changed

4 files changed

+7
-26
lines changed

src/bin/pg_upgrade/dump.c

-10
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ void
1818
generate_old_dump(void)
1919
{
2020
int dbnum;
21-
mode_t old_umask;
2221

2322
prep_status("Creating dump of global objects");
2423

@@ -33,13 +32,6 @@ generate_old_dump(void)
3332

3433
prep_status("Creating dump of database schemas\n");
3534

36-
/*
37-
* Set umask for this function, all functions it calls, and all
38-
* subprocesses/threads it creates. We can't use fopen_priv() as Windows
39-
* uses threads and umask is process-global.
40-
*/
41-
old_umask = umask(S_IRWXG | S_IRWXO);
42-
4335
/* create per-db dump files */
4436
for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
4537
{
@@ -74,8 +66,6 @@ generate_old_dump(void)
7466
while (reap_child(true) == true)
7567
;
7668

77-
umask(old_umask);
78-
7969
end_progress_output();
8070
check_ok();
8171
}

src/bin/pg_upgrade/file.c

-15
Original file line numberDiff line numberDiff line change
@@ -314,18 +314,3 @@ win32_pghardlink(const char *src, const char *dst)
314314
return 0;
315315
}
316316
#endif
317-
318-
319-
/* fopen() file with no group/other permissions */
320-
FILE *
321-
fopen_priv(const char *path, const char *mode)
322-
{
323-
mode_t old_umask = umask(S_IRWXG | S_IRWXO);
324-
FILE *fp;
325-
326-
fp = fopen(path, mode);
327-
328-
umask(old_umask); /* we assume this can't change errno */
329-
330-
return fp;
331-
}

src/bin/pg_upgrade/pg_upgrade.c

+4
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ main(int argc, char **argv)
7777
bool live_check = false;
7878

7979
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_upgrade"));
80+
81+
/* Ensure that all files created by pg_upgrade are non-world-readable */
82+
umask(S_IRWXG | S_IRWXO);
83+
8084
parseCommandLine(argc, argv);
8185

8286
get_restricted_token(os_info.progname);

src/bin/pg_upgrade/pg_upgrade.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,9 @@ void linkFile(const char *src, const char *dst,
374374
void rewriteVisibilityMap(const char *fromfile, const char *tofile,
375375
const char *schemaName, const char *relName);
376376
void check_hard_link(void);
377-
FILE *fopen_priv(const char *path, const char *mode);
377+
378+
/* fopen_priv() is no longer different from fopen() */
379+
#define fopen_priv(path, mode) fopen(path, mode)
378380

379381
/* function.c */
380382

0 commit comments

Comments
 (0)