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 8a662e9865d225b37b2c0536cb6e840e9331f0ea..def22c652116da60f80fccafda27e47dc53cddd6 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 f88e3d558f3e96086bb3077866f35c8dbb507638..f38bfacf02960e8ed8ebeb6f3dddbc1b71c574d0 100644 (file)
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -314,18 +314,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);                       /* we assume this can't change errno */
-
-       return fp;
-}

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index a67e484a8579dafabee41e9381396f060e650ee7..3f57a25b05f788bc2e5eade3ae9f1b4c02637b5f 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -77,6 +77,10 @@ main(int argc, char **argv)
        bool            live_check = false;
 
        set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_upgrade"));
+
+       /* 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 67f874b4f1edc0d1fc904da92d68dfc6a729a5d0..7e5e97129471cddd6922b6e5f76b9dc0c4be5cc7 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -374,7 +374,9 @@ void linkFile(const char *src, const char *dst,
 void rewriteVisibilityMap(const char *fromfile, const char *tofile,
                                         const char *schemaName, const char *relName);
 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 */