[PATCH] Fix possible string overflow with sscanf (xlog.c)

[Ranier Vilela]

Hi,
I know it's very hard, but is possible. Just someone with the knowledge to do.

Here a proof of concept:
#include <stdlib.h>
#include <string.h>

#define MAXPGPATH 256

int main(int argc, char ** argv)
{
        char            tbsoid[MAXPGPATH];
        char            str[MAXPGPATH];
        int                     ch,
                                prev_ch = -1,
                                i = 0,
                                n;
    FILE * lfp;

    lfp = fopen("c:\\tmp\\crash.dat", "rb");
        while ((ch = fgetc(lfp)) != EOF)
        {
                if ((ch == '\n' || ch == '\r') && prev_ch != '\\')
                {
                        str[i] = '\0';
                        if (sscanf(str, "%s %n", tbsoid, &n) != 1) {
               printf("tbsoid size=%u\n", strlen(tbsoid));
               printf("tbsoid=%s\n", tbsoid);
               exit(1);
            }
                        i = 0;
                        continue;
                }
                else if ((ch == '\n' || ch == '\r') && prev_ch == '\\')
                        str[i - 1] = ch;
                else
                        str[i++] = ch;
                prev_ch = ch;
        }
    fclose(lfp);
}

Overflow with (MAXPGPATH=256)
C:\usr\src\tests\scanf>sscanf3
tbsoid size=260
tbsoid=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxx

Now with patch:
C:\usr\src\tests\scanf>sscanf3
tbsoid size=255
tbsoid=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxx

The solution is simple, but clumsy. I hope that is enough.
sscanf(str, "%1023s %n", tbsoid, &n)

Best regards.
Ranier Vilela
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5f0ee50092..790e68ccf0 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -11382,7 +11382,7 @@ read_tablespace_map(List **tablespaces)
 		if ((ch == '\n' || ch == '\r') && prev_ch != '\\')
 		{
 			str[i] = '\0';
-			if (sscanf(str, "%s %n", tbsoid, &n) != 1)
+			if (sscanf(str, FMTPGPATH, tbsoid, &n) != 1)
 				ereport(FATAL,
 						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 						 errmsg("invalid data in file \"%s\"", TABLESPACE_MAP)));
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 743401cb96..03e0e3735e 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -84,6 +84,11 @@
  */
 #define MAXPGPATH		1024
 
+/*
+ * FMTPGPATH: standard format to scan pathname buffer, must follow MAXPGPATH
+ */
+#define FMTPGPATH               "%1023s %n"
+
 /*
  * PG_SOMAXCONN: maximum accept-queue length limit passed to
  * listen(2).  You'd think we should use SOMAXCONN from