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.
This commit is contained in:
Tom Lane 2011-12-07 13:34:13 -05:00
parent 0d9b09282f
commit 0d0ec527af
2 changed files with 46 additions and 18 deletions

View File

@ -47,7 +47,7 @@ static int64 sendDir(char *path, int basepathlen, bool sizeonly);
static void sendFile(char *readfilename, char *tarfilename, static void sendFile(char *readfilename, char *tarfilename,
struct stat * statbuf); struct stat * statbuf);
static void sendFileWithContent(const char *filename, const char *content); 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); struct stat * statbuf);
static void send_int8_string(StringInfoData *buf, int64 intval); static void send_int8_string(StringInfoData *buf, int64 intval);
static void SendBackupHeader(List *tablespaces); 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); snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name);
#if defined(HAVE_READLINK) || defined(WIN32) #if defined(HAVE_READLINK) || defined(WIN32)
rllen = readlink(fullpath, linkpath, sizeof(linkpath) - 1); rllen = readlink(fullpath, linkpath, sizeof(linkpath));
if (rllen < 0) if (rllen < 0)
{ {
ereport(WARNING, ereport(WARNING,
(errmsg("could not read symbolic link \"%s\": %m", fullpath))); (errmsg("could not read symbolic link \"%s\": %m",
fullpath)));
continue; continue;
} }
else if (rllen >= sizeof(linkpath)) else if (rllen >= sizeof(linkpath))
{ {
ereport(WARNING, ereport(WARNING,
(errmsg("symbolic link \"%s\" target is too long", fullpath))); (errmsg("symbolic link \"%s\" target is too long",
fullpath)));
continue; continue;
} }
linkpath[rllen] = '\0'; linkpath[rllen] = '\0';
@ -140,9 +142,9 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
tablespaces = lappend(tablespaces, ti); tablespaces = lappend(tablespaces, ti);
#else #else
/* /*
* If the platform does not have symbolic links, it should not be possible * If the platform does not have symbolic links, it should not be
* to have tablespaces - clearly somebody else created them. Warn about it * possible to have tablespaces - clearly somebody else created
* and ignore. * them. Warn about it and ignore.
*/ */
ereport(WARNING, ereport(WARNING,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@ -641,24 +643,45 @@ sendDir(char *path, int basepathlen, bool sizeonly)
continue; /* don't recurse into pg_xlog */ continue; /* don't recurse into pg_xlog */
} }
/* Allow symbolic links in pg_tblspc only */
if (strcmp(path, "./pg_tblspc") == 0 &&
#ifndef WIN32 #ifndef WIN32
if (S_ISLNK(statbuf.st_mode) && strcmp(path, "./pg_tblspc") == 0) S_ISLNK(statbuf.st_mode)
#else #else
if (pgwin32_is_junction(pathbuf) && strcmp(path, "./pg_tblspc") == 0) pgwin32_is_junction(pathbuf)
#endif #endif
)
{ {
/* Allow symbolic links in pg_tblspc */ #if defined(HAVE_READLINK) || defined(WIN32)
char linkpath[MAXPGPATH]; char linkpath[MAXPGPATH];
int rllen;
MemSet(linkpath, 0, sizeof(linkpath)); rllen = readlink(pathbuf, linkpath, sizeof(linkpath));
if (readlink(pathbuf, linkpath, sizeof(linkpath) - 1) == -1) if (rllen < 0)
ereport(ERROR, ereport(ERROR,
(errcode_for_file_access(), (errcode_for_file_access(),
errmsg("could not read symbolic link \"%s\": %m", errmsg("could not read symbolic link \"%s\": %m",
pathbuf))); pathbuf)));
if (rllen >= sizeof(linkpath))
ereport(ERROR,
(errmsg("symbolic link \"%s\" target is too long",
pathbuf)));
linkpath[rllen] = '\0';
if (!sizeonly) if (!sizeonly)
_tarWriteHeader(pathbuf + basepathlen + 1, linkpath, &statbuf); _tarWriteHeader(pathbuf + basepathlen + 1, linkpath, &statbuf);
size += 512; /* Size of the header just added */ 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)) else if (S_ISDIR(statbuf.st_mode))
{ {
@ -806,7 +829,8 @@ sendFile(char *readfilename, char *tarfilename, struct stat * statbuf)
static void static void
_tarWriteHeader(const char *filename, char *linktarget, struct stat * statbuf) _tarWriteHeader(const char *filename, const char *linktarget,
struct stat * statbuf)
{ {
char h[512]; char h[512];
int lastSum = 0; int lastSum = 0;
@ -854,7 +878,7 @@ _tarWriteHeader(const char *filename, char *linktarget, struct stat * statbuf)
{ {
/* Type - Symbolic link */ /* Type - Symbolic link */
sprintf(&h[156], "2"); sprintf(&h[156], "2");
strcpy(&h[157], linktarget); sprintf(&h[157], "%.99s", linktarget);
} }
else if (S_ISDIR(statbuf->st_mode)) else if (S_ISDIR(statbuf->st_mode))
/* Type - directory */ /* Type - directory */

View File

@ -274,7 +274,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
int rllen; int rllen;
/* /*
* Return empty string for our two default tablespace * Return empty string for our default tablespaces
*/ */
if (tablespaceOid == DEFAULTTABLESPACE_OID || if (tablespaceOid == DEFAULTTABLESPACE_OID ||
tablespaceOid == GLOBALTABLESPACE_OID) tablespaceOid == GLOBALTABLESPACE_OID)
@ -286,13 +286,16 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
* in pg_tblspc/<oid>. * in pg_tblspc/<oid>.
*/ */
snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%u", tablespaceOid); snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%u", tablespaceOid);
rllen =readlink(sourcepath, targetpath, sizeof(targetpath));
rllen = readlink(sourcepath, targetpath, sizeof(targetpath));
if (rllen < 0) if (rllen < 0)
ereport(ERROR, 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)) else if (rllen >= sizeof(targetpath))
ereport(ERROR, ereport(ERROR,
(errmsg("symbolic link \"%s\" target is too long", sourcepath))); (errmsg("symbolic link \"%s\" target is too long",
sourcepath)));
targetpath[rllen] = '\0'; targetpath[rllen] = '\0';
PG_RETURN_TEXT_P(cstring_to_text(targetpath)); PG_RETURN_TEXT_P(cstring_to_text(targetpath));
@ -300,6 +303,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("tablespaces are not supported on this platform"))); errmsg("tablespaces are not supported on this platform")));
PG_RETURN_NULL();
#endif #endif
} }