Harden astreamer tar parsing logic against archives it can't handle.

Previously, there was essentially no verification in this code that
the input is a tar file at all, let alone that it fits into the
subset of valid tar files that we can handle.  This was exposed by
the discovery that we couldn't handle files that FreeBSD's tar
makes, because it's fairly aggressive about converting sparse WAL
files into sparse tar entries.  To fix:

* Bail out if we find a pax extension header.  This covers the
sparse-file case, and also protects us against scenarios where
the pax header changes other file properties that we care about.
(Eventually we may extend the logic to actually handle such
headers, but that won't happen in time for v19.)

* Be more wary about tar file type codes in general: do not assume
that anything that's neither a directory nor a symlink must be a
regular file.  Instead, we just ignore entries that are none of the
three supported types.

* Apply pg_dump's isValidTarHeader to verify that a purported
header block is actually in tar format.  To make this possible,
move isValidTarHeader into src/port/tar.c, which is probably where
it should have been since that file was created.

I also took the opportunity to const-ify the arguments of
isValidTarHeader and tarChecksum, and to use symbols not hard-wired
constants inside tarChecksum.

Back-patch to v18 but not further.  Although this code exists inside
pg_basebackup in older branches, it's not really exposed in that
usage to tar files that weren't generated by our own code, so it
doesn't seem worth back-porting these changes across 3c9056981
and f80b09bac.  I did choose to include a back-patch of 5868372bb
into v18 though, to minimize cosmetic differences between these
two branches.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/3049460.1775067940@sss.pgh.pa.us>
Backpatch-through: 18

diff --git a/src/bin/pg_basebackup/astreamer_inject.c b/src/bin/pg_basebackup/astreamer_inject.c
index 14a96733badcfe666ccd8851afb1f3ef0352dc0f..a5dff0ac1c6d1039cf1b0f698d37b493b1542c81 100644 (file)
--- a/src/bin/pg_basebackup/astreamer_inject.c
+++ b/src/bin/pg_basebackup/astreamer_inject.c
@@ -224,8 +224,9 @@ astreamer_inject_file(astreamer *streamer, char *pathname, char *data,
        strlcpy(member.pathname, pathname, MAXPGPATH);
        member.size = len;
        member.mode = pg_file_create_mode;
+       member.is_regular = true;
        member.is_directory = false;
-       member.is_link = false;
+       member.is_symlink = false;
        member.linktarget[0] = '\0';
 
        /*

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 271a2c3e481ec8f99abe83929380ea983897e2b5..fecf6f2d1ce216d802410cf980c308238f1c96b9 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -44,6 +44,7 @@
 #include "pg_backup_archiver.h"
 #include "pg_backup_db.h"
 #include "pg_backup_utils.h"
+#include "pgtar.h"
 
 #define TEXT_DUMP_HEADER "--\n-- PostgreSQL database dump\n--\n\n"
 #define TEXT_DUMPALL_HEADER "--\n-- PostgreSQL database cluster dump\n--\n\n"
@@ -2372,7 +2373,7 @@ _discoverArchiveFormat(ArchiveHandle *AH)
                }
 
                if (!isValidTarHeader(AH->lookahead))
-                       pg_fatal("input file does not appear to be a valid archive");
+                       pg_fatal("input file does not appear to be a valid tar archive");
 
                AH->format = archTar;
        }

diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index 365073b3eae459008733541adced3924b9d5cbd5..9c3aca6543aa91690512b43b0f1d0a054481ee7a 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -465,8 +465,6 @@ extern void InitArchiveFmt_Null(ArchiveHandle *AH);
 extern void InitArchiveFmt_Directory(ArchiveHandle *AH);
 extern void InitArchiveFmt_Tar(ArchiveHandle *AH);
 
-extern bool isValidTarHeader(char *header);
-
 extern void ReconnectToServer(ArchiveHandle *AH, const char *dbname);
 extern void IssueCommandPerBlob(ArchiveHandle *AH, TocEntry *te,
                                                                const char *cmdBegin, const char *cmdEnd);

diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index d94d0de2a5d177829134115047c970523ea6b97b..a3879410c946a54141c791d2730e0c46ea7e9172 100644 (file)
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -984,31 +984,6 @@ tarPrintf(TAR_MEMBER *th, const char *fmt,...)
        return (int) cnt;
 }
 
-bool
-isValidTarHeader(char *header)
-{
-       int                     sum;
-       int                     chk = tarChecksum(header);
-
-       sum = read_tar_number(&header[TAR_OFFSET_CHECKSUM], 8);
-
-       if (sum != chk)
-               return false;
-
-       /* POSIX tar format */
-       if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar\0", 6) == 0 &&
-               memcmp(&header[TAR_OFFSET_VERSION], "00", 2) == 0)
-               return true;
-       /* GNU tar format */
-       if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar  \0", 8) == 0)
-               return true;
-       /* not-quite-POSIX format written by pre-9.3 pg_dump */
-       if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar00\0", 8) == 0)
-               return true;
-
-       return false;
-}
-
 /* Given the member, write the TAR header & copy the file */
 static void
 _tarAddFile(ArchiveHandle *AH, TAR_MEMBER *th)

diff --git a/src/bin/pg_verifybackup/astreamer_verify.c b/src/bin/pg_verifybackup/astreamer_verify.c
index 26c98186530c3e373e113dc658d79bafc066586b..99bd69ce7c9ef1bcdafcf8f7aa9822fabec7379b 100644 (file)
--- a/src/bin/pg_verifybackup/astreamer_verify.c
+++ b/src/bin/pg_verifybackup/astreamer_verify.c
@@ -165,7 +165,7 @@ member_verify_header(astreamer *streamer, astreamer_member *member)
        char            pathname[MAXPGPATH];
 
        /* We are only interested in normal files. */
-       if (member->is_directory || member->is_link)
+       if (!member->is_regular)
                return;
 
        /*

diff --git a/src/bin/pg_waldump/archive_waldump.c b/src/bin/pg_waldump/archive_waldump.c
index ff1cc3fa5f0dfb9ed8b86318303fdc5d7009d929..e4a4bf44a7eef840f7e69a4611d453e81a2aeaf0 100644 (file)
--- a/src/bin/pg_waldump/archive_waldump.c
+++ b/src/bin/pg_waldump/archive_waldump.c
@@ -815,7 +815,7 @@ member_is_wal_file(astreamer_waldump *mystreamer, astreamer_member *member,
        char       *filename;
 
        /* We are only interested in normal files */
-       if (member->is_directory || member->is_link)
+       if (!member->is_regular)
                return false;
 
        if (strlen(member->pathname) < XLOG_FNAME_LEN)

diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c
index 158e9a14f2c87b7aff4f586ae2ae4d18479637f2..0fca70a4f867dcd8d21a4c70b1627ca1dd2a2d3c 100644 (file)
--- a/src/fe_utils/astreamer_file.c
+++ b/src/fe_utils/astreamer_file.c
@@ -228,9 +228,13 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member,
                                mystreamer->filename[fnamelen - 1] = '\0';
 
                        /* Dispatch based on file type. */
-                       if (member->is_directory)
+                       if (member->is_regular)
+                               mystreamer->file =
+                                       create_file_for_extract(mystreamer->filename,
+                                                                                       member->mode);
+                       else if (member->is_directory)
                                extract_directory(mystreamer->filename, member->mode);
-                       else if (member->is_link)
+                       else if (member->is_symlink)
                        {
                                const char *linktarget = member->linktarget;
 
@@ -238,10 +242,6 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member,
                                        linktarget = mystreamer->link_map(linktarget);
                                extract_link(mystreamer->filename, linktarget);
                        }
-                       else
-                               mystreamer->file =
-                                       create_file_for_extract(mystreamer->filename,
-                                                                                       member->mode);
 
                        /* Report output file change. */
                        if (mystreamer->report_output_file)

diff --git a/src/fe_utils/astreamer_tar.c b/src/fe_utils/astreamer_tar.c
index 3b094fc0328df47bd82d4021521d3aa0c5a64558..f584665057dabfdbd22dc4588be61c3ea8929eef 100644 (file)
--- a/src/fe_utils/astreamer_tar.c
+++ b/src/fe_utils/astreamer_tar.c
@@ -260,7 +260,8 @@ astreamer_tar_parser_content(astreamer *streamer, astreamer_member *member,
  * Parse a file header within a tar stream.
  *
  * The return value is true if we found a file header and passed it on to the
- * next astreamer; it is false if we have reached the archive trailer.
+ * next astreamer; it is false if we have found the archive trailer.
+ * We throw error if we see invalid data.
  */
 static bool
 astreamer_tar_header(astreamer_tar_parser *mystreamer)
@@ -272,6 +273,9 @@ astreamer_tar_header(astreamer_tar_parser *mystreamer)
 
        Assert(mystreamer->base.bbs_buffer.len == TAR_BLOCK_SIZE);
 
+       /* Zero out fields of *member, just for consistency. */
+       memset(member, 0, sizeof(astreamer_member));
+
        /* Check whether we've got a block of all zero bytes. */
        for (i = 0; i < TAR_BLOCK_SIZE; ++i)
        {
@@ -289,6 +293,12 @@ astreamer_tar_header(astreamer_tar_parser *mystreamer)
        if (!has_nonzero_byte)
                return false;
 
+       /*
+        * Verify that we have a reasonable-looking header.
+        */
+       if (!isValidTarHeader(buffer))
+               pg_fatal("input file does not appear to be a valid tar archive");
+
        /*
         * Parse key fields out of the header.
         */
@@ -299,12 +309,28 @@ astreamer_tar_header(astreamer_tar_parser *mystreamer)
        member->mode = read_tar_number(&buffer[TAR_OFFSET_MODE], 8);
        member->uid = read_tar_number(&buffer[TAR_OFFSET_UID], 8);
        member->gid = read_tar_number(&buffer[TAR_OFFSET_GID], 8);
-       member->is_directory =
-               (buffer[TAR_OFFSET_TYPEFLAG] == TAR_FILETYPE_DIRECTORY);
-       member->is_link =
-               (buffer[TAR_OFFSET_TYPEFLAG] == TAR_FILETYPE_SYMLINK);
-       if (member->is_link)
-               strlcpy(member->linktarget, &buffer[TAR_OFFSET_LINKNAME], 100);
+
+       switch (buffer[TAR_OFFSET_TYPEFLAG])
+       {
+               case TAR_FILETYPE_PLAIN:
+               case TAR_FILETYPE_PLAIN_OLD:
+                       member->is_regular = true;
+                       break;
+               case TAR_FILETYPE_DIRECTORY:
+                       member->is_directory = true;
+                       break;
+               case TAR_FILETYPE_SYMLINK:
+                       member->is_symlink = true;
+                       strlcpy(member->linktarget, &buffer[TAR_OFFSET_LINKNAME], 100);
+                       break;
+               case TAR_FILETYPE_PAX_EXTENDED:
+               case TAR_FILETYPE_PAX_EXTENDED_GLOBAL:
+                       pg_fatal("pax extensions to tar format are not supported");
+                       break;
+               default:
+                       /* For special filetypes, set none of the three is_xxx flags */
+                       break;
+       }
 
        /* Compute number of padding bytes. */
        mystreamer->pad_bytes_expected = tarPaddingBytesRequired(member->size);

diff --git a/src/include/fe_utils/astreamer.h b/src/include/fe_utils/astreamer.h
index f370ef62720edbc4b808f2310c29a49112a30687..8329e4efbc5222c40333037df69b7419cd3d0872 100644 (file)
--- a/src/include/fe_utils/astreamer.h
+++ b/src/include/fe_utils/astreamer.h
@@ -83,8 +83,10 @@ typedef struct
        mode_t          mode;
        uid_t           uid;
        gid_t           gid;
+       /* note: special filetypes will set none of these flags */
+       bool            is_regular;
        bool            is_directory;
-       bool            is_link;
+       bool            is_symlink;
        char            linktarget[MAXPGPATH];
 } astreamer_member;
 

diff --git a/src/include/pgtar.h b/src/include/pgtar.h
index eb93bdef5c4494c2d6f5110c49a20a90a02c0b2f..323ba051f8b20c2e7bd011b3dd014739b978ef5e 100644 (file)
--- a/src/include/pgtar.h
+++ b/src/include/pgtar.h
@@ -55,11 +55,15 @@ enum tarHeaderOffset
        /* last 12 bytes of the 512-byte block are unassigned */
 };
 
+/* See POSIX (not all the standard file type codes are listed here) */
 enum tarFileType
 {
        TAR_FILETYPE_PLAIN = '0',
+       TAR_FILETYPE_PLAIN_OLD = '\0',  /* backwards compatibility, per POSIX */
        TAR_FILETYPE_SYMLINK = '2',
        TAR_FILETYPE_DIRECTORY = '5',
+       TAR_FILETYPE_PAX_EXTENDED = 'x',
+       TAR_FILETYPE_PAX_EXTENDED_GLOBAL = 'g',
 };
 
 extern enum tarError tarCreateHeader(char *h, const char *filename,
@@ -68,7 +72,8 @@ extern enum tarError tarCreateHeader(char *h, const char *filename,
                                                                         time_t mtime);
 extern uint64 read_tar_number(const char *s, int len);
 extern void print_tar_number(char *s, int len, uint64 val);
-extern int     tarChecksum(char *header);
+extern int     tarChecksum(const char *header);
+extern bool isValidTarHeader(const char *header);
 
 /*
  * Compute the number of padding bytes required for an entry in a tar

diff --git a/src/port/tar.c b/src/port/tar.c
index 592b4fb7b0f4e08fb9c9efdee6439e54661aa600..fee9dbbf5be2d70e8f0ada2d82aeace301873909 100644 (file)
--- a/src/port/tar.c
+++ b/src/port/tar.c
@@ -87,7 +87,7 @@ read_tar_number(const char *s, int len)
  * be 512 bytes, per the tar standard.
  */
 int
-tarChecksum(char *header)
+tarChecksum(const char *header)
 {
        int                     i,
                                sum;
@@ -95,15 +95,44 @@ tarChecksum(char *header)
        /*
         * Per POSIX, the checksum is the simple sum of all bytes in the header,
         * treating the bytes as unsigned, and treating the checksum field (at
-        * offset 148) as though it contained 8 spaces.
+        * offset TAR_OFFSET_CHECKSUM) as though it contained 8 spaces.
         */
        sum = 8 * ' ';                          /* presumed value for checksum field */
-       for (i = 0; i < 512; i++)
-               if (i < 148 || i >= 156)
+       for (i = 0; i < TAR_BLOCK_SIZE; i++)
+               if (i < TAR_OFFSET_CHECKSUM || i >= TAR_OFFSET_CHECKSUM + 8)
                        sum += 0xFF & header[i];
        return sum;
 }
 
+/*
+ * Check validity of a tar header (assumed to be 512 bytes long).
+ * We verify the checksum and the magic number / version.
+ */
+bool
+isValidTarHeader(const char *header)
+{
+       int                     sum;
+       int                     chk = tarChecksum(header);
+
+       sum = read_tar_number(&header[TAR_OFFSET_CHECKSUM], 8);
+
+       if (sum != chk)
+               return false;
+
+       /* POSIX tar format */
+       if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar\0", 6) == 0 &&
+               memcmp(&header[TAR_OFFSET_VERSION], "00", 2) == 0)
+               return true;
+       /* GNU tar format */
+       if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar  \0", 8) == 0)
+               return true;
+       /* not-quite-POSIX format written by pre-9.3 pg_dump */
+       if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar00\0", 8) == 0)
+               return true;
+
+       return false;
+}
+
 
 /*
  * Fill in the buffer pointed to by h with a tar format header. This buffer