Fix to CVE-2006-0553 for 8.1.1

Started by Albert Chinabout 20 years ago4 messageshackers
Jump to latest
#1Albert Chin
pgsql-hackers@mlists.thewrittenword.com

Does the patch below look like the correct fix to CVE-2006-0553 if
running 8.1.1? I just scanned cvs log from the 8.1 branch, looking for
CVE-2006-0553 and picked out the diffs.

--
albert chin (china@thewrittenword.com)

-- snip snip
Index: src/backend/commands/variable.c
===================================================================
--- src/backend/commands/variable.c.orig	2005-11-22 12:23:08.000000000 -0600
+++ src/backend/commands/variable.c	2006-02-19 15:24:40.540106000 -0600
@@ -586,7 +586,9 @@
  * by the numeric oid, followed by a comma, followed by the role name.
  * This cannot be confused with a plain role name because of the NAMEDATALEN
  * limit on names, so we can tell whether we're being passed an initial
- * role name or a saved/restored value.
+ * role name or a saved/restored value.  (NOTE: we rely on guc.c to have
+ * properly truncated any incoming value, but not to truncate already-stored
+ * values.  See GUC_IS_NAME processing.)
  */
 extern char *session_authorization_string;		/* in guc.c */
Index: src/include/utils/guc_tables.h
===================================================================
--- src/include/utils/guc_tables.h.orig	2005-07-14 00:13:44.000000000 -0500
+++ src/include/utils/guc_tables.h	2006-02-19 15:29:15.187973000 -0600
@@ -126,6 +126,7 @@
 #define GUC_DISALLOW_IN_FILE	0x0040	/* can't set in postgresql.conf */
 #define GUC_CUSTOM_PLACEHOLDER	0x0080	/* placeholder for custom variable */
 #define GUC_SUPERUSER_ONLY		0x0100	/* show only to superusers */
+#define GUC_IS_NAME			0x0200  /* limit string to NAMEDATALEN-1 */
 /* bit values in status field */
 #define GUC_HAVE_TENTATIVE	0x0001		/* tentative value is defined */
Index: src/backend/utils/misc/guc.c
===================================================================
--- src/backend/utils/misc/guc.c.orig	2005-11-22 12:23:24.000000000 -0600
+++ src/backend/utils/misc/guc.c	2006-02-19 15:30:21.625766000 -0600
@@ -48,6 +48,7 @@
 #include "optimizer/prep.h"
 #include "parser/parse_expr.h"
 #include "parser/parse_relation.h"
+#include "parser/scansup.h"
 #include "postmaster/autovacuum.h"
 #include "postmaster/bgwriter.h"
 #include "postmaster/syslogger.h"
@@ -1662,7 +1663,7 @@
 		{"client_encoding", PGC_USERSET, CLIENT_CONN_LOCALE,
 			gettext_noop("Sets the client's character set encoding."),
 			NULL,
-			GUC_REPORT
+			GUC_IS_NAME | GUC_REPORT
 		},
 		&client_encoding_string,
 		"SQL_ASCII", assign_client_encoding, NULL
@@ -1742,7 +1743,8 @@
 	{
 		{"default_tablespace", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Sets the default tablespace to create tables and indexes in."),
-			gettext_noop("An empty string selects the database's default tablespace.")
+			gettext_noop("An empty string selects the database's default tablespace."),
+		 	GUC_IS_NAME
 		},
 		&default_tablespace,
 		"", assign_default_tablespace, NULL
@@ -1900,7 +1902,7 @@
 		{"server_encoding", PGC_INTERNAL, CLIENT_CONN_LOCALE,
 			gettext_noop("Sets the server (database) character set encoding."),
 			NULL,
-			GUC_REPORT | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_IS_NAME | GUC_REPORT | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
 		},
 		&server_encoding_string,
 		"SQL_ASCII", NULL, NULL
@@ -1922,7 +1924,7 @@
 		{"role", PGC_USERSET, UNGROUPED,
 			gettext_noop("Sets the current role."),
 			NULL,
-			GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_IS_NAME | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
 		},
 		&role_string,
 		"none", assign_role, show_role
@@ -1933,7 +1935,7 @@
 		{"session_authorization", PGC_USERSET, UNGROUPED,
 			gettext_noop("Sets the session user name."),
 			NULL,
-			GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_IS_NAME | GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
 		},
 		&session_authorization_string,
 		NULL, assign_session_authorization, show_session_authorization
@@ -3934,6 +3936,12 @@
 					newval = guc_strdup(elevel, value);
 					if (newval == NULL)
 						return false;
+					/*
+					 * The only sort of "parsing" check we need to do is
+					 * apply truncation if GUC_IS_NAME.
+					 */
+					if (conf->gen.flags & GUC_IS_NAME)
+						truncate_identifier(newval, strlen(newval), true);
 				}
 				else if (conf->reset_val)
 				{
Index: src/backend/utils/mb/encnames.c
===================================================================
--- src/backend/utils/mb/encnames.c.orig	2005-10-14 21:49:33.000000000 -0500
+++ src/backend/utils/mb/encnames.c	2006-02-19 15:35:15.559558000 -0600
@@ -449,7 +449,7 @@
 	if (name == NULL || *name == '\0')
 		return NULL;

- if (strlen(name) > NAMEDATALEN)
+ if (strlen(name) >= NAMEDATALEN)
{
#ifdef FRONTEND
fprintf(stderr, "encoding name too long\n");

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Albert Chin (#1)
Re: Fix to CVE-2006-0553 for 8.1.1

Albert Chin <pgsql-hackers@mlists.thewrittenword.com> writes:

Does the patch below look like the correct fix to CVE-2006-0553 if
running 8.1.1?

Why in the world would you not install 8.1.3 instead? Or are you hoping
to get burnt by one of the *other* bugs in 8.1.1?

regards, tom lane

#3Albert Chin
pgsql-hackers@mlists.thewrittenword.com
In reply to: Tom Lane (#2)
Re: Fix to CVE-2006-0553 for 8.1.1

On Sun, Feb 19, 2006 at 05:14:32PM -0500, Tom Lane wrote:

Albert Chin <pgsql-hackers@mlists.thewrittenword.com> writes:

Does the patch below look like the correct fix to CVE-2006-0553 if
running 8.1.1?

Why in the world would you not install 8.1.3 instead? Or are you hoping
to get burnt by one of the *other* bugs in 8.1.1?

We've already deployed 8.1.1 to some customers. We will offer 8.1.3
but if they want to upgrade 8.1.1 to fix the security issue, we want
this to be an option.

--
albert chin (china@thewrittenword.com)

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Albert Chin (#3)
Re: Fix to CVE-2006-0553 for 8.1.1

Albert Chin <pgsql-hackers@mlists.thewrittenword.com> writes:

On Sun, Feb 19, 2006 at 05:14:32PM -0500, Tom Lane wrote:

Why in the world would you not install 8.1.3 instead? Or are you hoping
to get burnt by one of the *other* bugs in 8.1.1?

We've already deployed 8.1.1 to some customers. We will offer 8.1.3
but if they want to upgrade 8.1.1 to fix the security issue, we want
this to be an option.

You want an option to leave data-loss-causing bugs unfixed, eh? Make
sure you make those customers sign a disclaimer that it's their fault
not yours when the ReadBuffer bug eats their data.

regards, tom lane