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 2f4ae3b53db5a5e4425ee9191db9c95eafa4df16..c12e936f98eefbed9d21583aec0c6a20acb45086 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -47,7 +47,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);
@@ -118,17 +118,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';
@@ -140,9 +142,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),
@@ -641,24 +643,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))
        {
@@ -806,7 +829,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;
@@ -854,7 +878,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 */

diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 478f203273fac7ed677e666148a4c5259717022f..7a2e0c8e6224b68a9b54f57970d39deaf1ea048e 100644 (file)
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -274,7 +274,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
    int     rllen;
 
    /*
-    * Return empty string for our two default tablespace
+    * Return empty string for our default tablespaces
     */
    if (tablespaceOid == DEFAULTTABLESPACE_OID ||
        tablespaceOid == GLOBALTABLESPACE_OID)
@@ -286,13 +286,16 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
     * in pg_tblspc/<oid>.
     */
    snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%u", tablespaceOid);
-   rllen =readlink(sourcepath, targetpath, sizeof(targetpath));
+
+   rllen = readlink(sourcepath, targetpath, sizeof(targetpath));
    if (rllen < 0)
        ereport(ERROR,
-               (errmsg("could not read symbolic link \"%s\": %m", sourcepath)));
+               (errmsg("could not read symbolic link \"%s\": %m",
+                       sourcepath)));
    else if (rllen >= sizeof(targetpath))
        ereport(ERROR,
-               (errmsg("symbolic link \"%s\" target is too long", sourcepath)));
+               (errmsg("symbolic link \"%s\" target is too long",
+                       sourcepath)));
    targetpath[rllen] = '\0';
 
    PG_RETURN_TEXT_P(cstring_to_text(targetpath));
@@ -300,6 +303,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
    ereport(ERROR,
            (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
             errmsg("tablespaces are not supported on this platform")));
+   PG_RETURN_NULL();
 #endif
 }