]> granicus.if.org Git - postgresql/commitdiff
Ensure that all temp files made during pg_upgrade are non-world-readable.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 5 Feb 2018 15:58:27 +0000 (10:58 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 5 Feb 2018 15:58:27 +0000 (10:58 -0500)
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

contrib/pg_upgrade/dump.c
contrib/pg_upgrade/file.c
contrib/pg_upgrade/pg_upgrade.c
contrib/pg_upgrade/pg_upgrade.h

index 7a0e3421486fd2cccb8c21e243bd6d1c78867c17..cc253ebf3ddd867bec9473e40d7242782e148caa 100644 (file)
@@ -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();
 }
index 4f0f1ae4a8fb523aeeba0f1d8b58732201e3bba2..2a4f50d9b4ed6d300ecd2e9cbb353b133f7e4f5b 100644 (file)
@@ -239,17 +239,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;
-}
index 96221240a74556cce87be9270531199e7d6db375..24d9d51e54d853f9417050ee477613c4573c3970 100644 (file)
@@ -83,6 +83,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);
index aec1f54b15d6882b67c195156c681fdceb2aa0f7..52a68e7081b0c5324addfbf7bc3f4fb4b5fac13a 100644 (file)
@@ -385,7 +385,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 */