Fix corner cases in readlink() usage.

Make sure all calls are protected by HAVE_READLINK, and get the buffer
overflow tests right.  Be a bit more paranoid about string length in
_tarWriteHeader(), too.

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 73a7f17149e720c7d6a143f1567ffb859b9be993..47e7cb4bd2f0bfa94a960e640e30cc9cac0935b9 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -46,7 +46,7 @@ static int64 sendDir(char *path, int basepathlen, bool sizeonly);
 static void sendFile(char *readfilename, char *tarfilename,
                 struct stat * statbuf);
 static void sendFileWithContent(const char *filename, const char *content);
-static void _tarWriteHeader(const char *filename, char *linktarget,
+static void _tarWriteHeader(const char *filename, const char *linktarget,
                                struct stat * statbuf);
 static void send_int8_string(StringInfoData *buf, int64 intval);
 static void SendBackupHeader(List *tablespaces);
@@ -117,17 +117,19 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
                        snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name);
 
 #if defined(HAVE_READLINK) || defined(WIN32)
-                       rllen = readlink(fullpath, linkpath, sizeof(linkpath) - 1);
+                       rllen = readlink(fullpath, linkpath, sizeof(linkpath));
                        if (rllen < 0)
                        {
                                ereport(WARNING,
-                                               (errmsg("could not read symbolic link \"%s\": %m", fullpath)));
+                                               (errmsg("could not read symbolic link \"%s\": %m",
+                                                               fullpath)));
                                continue;
                        }
                        else if (rllen >= sizeof(linkpath))
                        {
                                ereport(WARNING,
-                                               (errmsg("symbolic link \"%s\" target is too long", fullpath)));
+                                               (errmsg("symbolic link \"%s\" target is too long",
+                                                               fullpath)));
                                continue;
                        }
                        linkpath[rllen] = '\0';
@@ -139,9 +141,9 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
                        tablespaces = lappend(tablespaces, ti);
 #else
                        /*
-                        * If the platform does not have symbolic links, it should not be possible
-                        * to have tablespaces - clearly somebody else created them. Warn about it
-                        * and ignore.
+                        * If the platform does not have symbolic links, it should not be
+                        * possible to have tablespaces - clearly somebody else created
+                        * them. Warn about it and ignore.
                         */
                        ereport(WARNING,
                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -635,24 +637,45 @@ sendDir(char *path, int basepathlen, bool sizeonly)
                        continue;                       /* don't recurse into pg_xlog */
                }
 
+               /* Allow symbolic links in pg_tblspc only */
+               if (strcmp(path, "./pg_tblspc") == 0 &&
 #ifndef WIN32
-               if (S_ISLNK(statbuf.st_mode) && strcmp(path, "./pg_tblspc") == 0)
+                                S_ISLNK(statbuf.st_mode)
 #else
-               if (pgwin32_is_junction(pathbuf) && strcmp(path, "./pg_tblspc") == 0)
+                                pgwin32_is_junction(pathbuf)
 #endif
+                       )
                {
-                       /* Allow symbolic links in pg_tblspc */
+#if defined(HAVE_READLINK) || defined(WIN32)
                        char            linkpath[MAXPGPATH];
+                       int                     rllen;
 
-                       MemSet(linkpath, 0, sizeof(linkpath));
-                       if (readlink(pathbuf, linkpath, sizeof(linkpath) - 1) == -1)
+                       rllen = readlink(pathbuf, linkpath, sizeof(linkpath));
+                       if (rllen < 0)
                                ereport(ERROR,
                                                (errcode_for_file_access(),
                                                 errmsg("could not read symbolic link \"%s\": %m",
                                                                pathbuf)));
+                       if (rllen >= sizeof(linkpath))
+                               ereport(ERROR,
+                                               (errmsg("symbolic link \"%s\" target is too long",
+                                                               pathbuf)));
+                       linkpath[rllen] = '\0';
+
                        if (!sizeonly)
                                _tarWriteHeader(pathbuf + basepathlen + 1, linkpath, &statbuf);
                        size += 512;            /* Size of the header just added */
+#else
+                       /*
+                        * If the platform does not have symbolic links, it should not be
+                        * possible to have tablespaces - clearly somebody else created
+                        * them. Warn about it and ignore.
+                        */
+                       ereport(WARNING,
+                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("tablespaces are not supported on this platform")));
+                       continue;
+#endif /* HAVE_READLINK */
                }
                else if (S_ISDIR(statbuf.st_mode))
                {
@@ -800,7 +823,8 @@ sendFile(char *readfilename, char *tarfilename, struct stat * statbuf)
 
 
 static void
-_tarWriteHeader(const char *filename, char *linktarget, struct stat * statbuf)
+_tarWriteHeader(const char *filename, const char *linktarget,
+                               struct stat * statbuf)
 {
        char            h[512];
        int                     lastSum = 0;
@@ -848,7 +872,7 @@ _tarWriteHeader(const char *filename, char *linktarget, struct stat * statbuf)
        {
                /* Type - Symbolic link */
                sprintf(&h[156], "2");
-               strcpy(&h[157], linktarget);
+               sprintf(&h[157], "%.99s", linktarget);
        }
        else if (S_ISDIR(statbuf->st_mode))
                /* Type - directory */