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 76a7650c4 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

diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index dd97891a082bae11f84c78a7bd17cf154bbbde94..66b66d2a5fc9367c240251e5a31a2fa9c3c8a3cf 100644 (file)
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -18,7 +18,6 @@ void
 generate_old_dump(void)
 {
    int         dbnum;
-   mode_t      old_umask;
 
    prep_status("Creating dump of global objects");
 
@@ -33,13 +32,6 @@ generate_old_dump(void)
 
    prep_status("Creating dump of database schemas\n");
 
-   /*
-    * Set umask for this function, all functions it calls, and all
-    * subprocesses/threads it creates.  We can't use fopen_priv() as Windows
-    * uses threads and umask is process-global.
-    */
-   old_umask = umask(S_IRWXG | S_IRWXO);
-
    /* create per-db dump files */
    for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
    {
@@ -74,8 +66,6 @@ generate_old_dump(void)
    while (reap_child(true) == true)
        ;
 
-   umask(old_umask);
-
    end_progress_output();
    check_ok();
 }

diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index c84783c9de61dab8d016c2d272c9380f8b417882..a433c02724eaa09ce06fa550ea1fe3ee0f5c95ed 100644 (file)
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -238,17 +238,3 @@ win32_pghardlink(const char *src, const char *dst)
        return 0;
 }
 #endif
-
-
-/* fopen() file with no group/other permissions */
-FILE *
-fopen_priv(const char *path, const char *mode)
-{
-   mode_t      old_umask = umask(S_IRWXG | S_IRWXO);
-   FILE       *fp;
-
-   fp = fopen(path, mode);
-   umask(old_umask);
-
-   return fp;
-}

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 741a37c3825fee569b51319bd335f7df2792746c..ce0ce04a3e9b5aa86f20804e8e52ed201e69929f 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -74,6 +74,9 @@ main(int argc, char **argv)
    char       *deletion_script_file_name = NULL;
    bool        live_check = false;
 
+   /* Ensure that all files created by pg_upgrade are non-world-readable */
+   umask(S_IRWXG | S_IRWXO);
+
    parseCommandLine(argc, argv);
 
    get_restricted_token(os_info.progname);

diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index aa248027bd955f0b538da2591b61cdd1e0491f27..6f41f7de3ba11d5c2df2b8a2ece55cf1c234cfdb 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -400,7 +400,9 @@ const char *linkAndUpdateFile(pageCnvCtx *pageConverter, const char *src,
                  const char *dst);
 
 void       check_hard_link(void);
-FILE      *fopen_priv(const char *path, const char *mode);
+
+/* fopen_priv() is no longer different from fopen() */
+#define fopen_priv(path, mode) fopen(path, mode)
 
 /* function.c */