Fix a read of uninitialized memory in next_token() of hba.c, spotted via

valgrind: a buffer passed to strncmp() had to be NUL-terminated. Original
report and patch from Dennis Bjorkland, some cleanup by Andrew Dunstan,
and finally some editorializing from Neil Conway.

src/backend/libpq/hba.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/libpq/hba.c,v 1.119 2003/12/25 03:44:04 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/libpq/hba.c,v 1.120 2004/02/02 16:58:30 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,12 +37,23 @@
 #include "storage/fd.h"
 
 
-#define IDENT_USERNAME_MAX 512
 /* Max size of username ident server can return */
+#define IDENT_USERNAME_MAX 512
+
+/* Standard TCP port number for Ident service.  Assigned by IANA */
+#define IDENT_PORT 113
+
+/* Name of the config file  */
+#define CONF_FILE "pg_hba.conf"
+
+/* Name of the usermap file */
+#define USERMAP_FILE "pg_ident.conf"
 
 /* This is used to separate values in multi-valued column strings */
 #define MULTI_VALUE_SEP "\001"
 
+#define MAX_TOKEN	256
+
 /*
  * These variables hold the pre-parsed contents of the hba and ident
  * configuration files.  Each is a list of sublists, one sublist for
@@ -80,19 +91,19 @@ pg_isblank(const char c)
 
 
 /*
- *	 Grab one token out of fp. Tokens are strings of non-blank
- *	 characters bounded by blank characters, beginning of line, and
- *	 end of line. Blank means space or tab. Return the token as
- *	 *buf. Leave file positioned to character immediately after the
- *	 token or EOF, whichever comes first. If no more tokens on line,
- *	 return null string as *buf and position file to beginning of
- *	 next line or EOF, whichever comes first. Allow spaces in quoted
- *	 strings. Terminate on unquoted commas. Handle comments. Treat
- *   unquoted keywords that might be user names or database names 
- *   specially, by appending a newline to them.
+ * Grab one token out of fp. Tokens are strings of non-blank
+ * characters bounded by blank characters, beginning of line, and
+ * end of line. Blank means space or tab. Return the token as
+ * *buf. Leave file positioned at the character immediately after the
+ * token or EOF, whichever comes first. If no more tokens on line,
+ * return empty string as *buf and position the file to the beginning
+ * of the next line or EOF, whichever comes first. Allow spaces in
+ * quoted strings. Terminate on unquoted commas. Handle
+ * comments. Treat unquoted keywords that might be user names or
+ * database names specially, by appending a newline to them.
  */
-void
-next_token(FILE *fp, char *buf, const int bufsz)
+static void
+next_token(FILE *fp, char *buf, int bufsz)
 {
 	int			c;
 	char	   *start_buf = buf;
@@ -101,88 +112,89 @@ next_token(FILE *fp, char *buf, const int bufsz)
 	bool		was_quote = false;
 	bool        saw_quote = false;
 
+	Assert(end_buf > start_buf);
+
 	/* Move over initial whitespace and commas */
 	while ((c = getc(fp)) != EOF && (pg_isblank(c) || c == ','))
 		;
 
-	if (c != EOF && c != '\n')
+	if (c == EOF || c == '\n')
 	{
-		/*
-		 * Build a token in buf of next characters up to EOF, EOL,
-		 * unquoted comma, or unquoted whitespace.
-		 */
-		while (c != EOF && c != '\n' &&
-			   (!pg_isblank(c) || in_quote == true))
-		{
-			/* skip comments to EOL */
-			if (c == '#' && !in_quote)
-			{
-				while ((c = getc(fp)) != EOF && c != '\n')
-					;
-				/* If only comment, consume EOL too; return EOL */
-				if (c != EOF && buf == start_buf)
-					c = getc(fp);
-				break;
-			}
+		*buf = '\0';
+		return;
+	}
 
-			if (buf >= end_buf)
-			{
-				ereport(LOG,
-						(errcode(ERRCODE_CONFIG_FILE_ERROR),
-						 errmsg("authentication file token too long, skipping: \"%s\"",
-								buf)));
-				/* Discard remainder of line */
-				while ((c = getc(fp)) != EOF && c != '\n')
-					;
-				buf[0] = '\0';
-				break;
-			}
+	/*
+	 * Build a token in buf of next characters up to EOF, EOL,
+	 * unquoted comma, or unquoted whitespace.
+	 */
+	while (c != EOF && c != '\n' &&
+		   (!pg_isblank(c) || in_quote == true))
+	{
+		/* skip comments to EOL */
+		if (c == '#' && !in_quote)
+		{
+			while ((c = getc(fp)) != EOF && c != '\n')
+				;
+			/* If only comment, consume EOL too; return EOL */
+			if (c != EOF && buf == start_buf)
+				c = getc(fp);
+			break;
+		}
 
-			if (c != '"' || (c == '"' && was_quote))
-				*buf++ = c;
+		if (buf >= end_buf)
+		{
+			ereport(LOG,
+					(errcode(ERRCODE_CONFIG_FILE_ERROR),
+					 errmsg("authentication file token too long, skipping: \"%s\"",
+							buf)));
+			/* Discard remainder of line */
+			while ((c = getc(fp)) != EOF && c != '\n')
+				;
+			buf[0] = '\0';
+			break;
+		}
 
-			/* We pass back the comma so the caller knows there is more */
-			if ((pg_isblank(c) || c == ',') && !in_quote)
-				break;
+		if (c != '"' || (c == '"' && was_quote))
+			*buf++ = c;
 
-			/* Literal double-quote is two double-quotes */
-			if (in_quote && c == '"')
-				was_quote = !was_quote;
-			else
-				was_quote = false;
+		/* We pass back the comma so the caller knows there is more */
+		if ((pg_isblank(c) || c == ',') && !in_quote)
+			break;
 
-			if (c == '"')
-			{
-				in_quote = !in_quote;
-				saw_quote = true;
-			}
+		/* Literal double-quote is two double-quotes */
+		if (in_quote && c == '"')
+			was_quote = !was_quote;
+		else
+			was_quote = false;
 
-			c = getc(fp);
+		if (c == '"')
+		{
+			in_quote = !in_quote;
+			saw_quote = true;
 		}
 
-		/*
-		 * Put back the char right after the token (critical in case it is
-		 * EOL, since we need to detect end-of-line at next call).
-		 */
-		if (c != EOF)
-			ungetc(c, fp);
+		c = getc(fp);
 	}
 
+	/*
+	 * Put back the char right after the token (critical in case it is
+	 * EOL, since we need to detect end-of-line at next call).
+	 */
+	if (c != EOF)
+		ungetc(c, fp);
+
+	*buf = '\0';
 
-	if ( !saw_quote && 
-	     (
-			 strncmp(start_buf,"all",3) == 0  ||
-			 strncmp(start_buf,"sameuser",8) == 0  ||
-			 strncmp(start_buf,"samegroup",9) == 0 
-		 )
-		)
+	if (!saw_quote && 
+	     (strcmp(start_buf, "all") == 0 ||
+		  strcmp(start_buf, "sameuser") == 0 ||
+		  strcmp(start_buf, "samegroup") == 0))
 	{
 		/* append newline to a magical keyword */
 		*buf++ = '\n';
+		*buf = '\0';
 	}
-
-	*buf = '\0';
-
 }
 
 /*

src/include/libpq/hba.h

@@ -4,7 +4,7 @@
  *	  Interface to hba.c
  *
  *
- * $PostgreSQL: pgsql/src/include/libpq/hba.h,v 1.34 2003/11/29 22:41:03 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/libpq/hba.h,v 1.35 2004/02/02 16:58:30 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,15 +17,6 @@
 
 #include "nodes/pg_list.h"
 
-#define CONF_FILE "pg_hba.conf"
- /* Name of the config file  */
-
-#define USERMAP_FILE "pg_ident.conf"
- /* Name of the usermap file */
-
-#define IDENT_PORT 113
- /* Standard TCP port number for Ident service.  Assigned by IANA */
-
 typedef enum UserAuth
 {
 	uaReject,
@@ -43,9 +34,6 @@ typedef enum UserAuth
 
 typedef struct Port hbaPort;
 
-#define MAX_TOKEN	256
-
-extern void next_token(FILE *fp, char *buf, const int bufsz);
 extern List **get_user_line(const char *user);
 extern void load_hba(void);
 extern void load_ident(void);