Cursor support buffer patch

Ian Lance Taylor <ian@airs.com> writes:

The code assumed that there would be a '\0' in buf after storing the
characters in new->refname, but it did nothing to ensure that.

Good catch.

I can't convince myself that this code does not have the possibility
of buffer overflow.

It obviously does; the fixed-size buffer should be replaced by a
PLpgSQL_dstring, probably. I don't much like the fixed-size
fieldnames[] buffers elsewhere in that file, either.

regards, tom lane

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

Here is a small patch for the cursor support which Jan recently added.
The code assumed that there would be a '\0' in buf after storing the
characters in new->refname, but it did nothing to ensure that.

I can't convince myself that this code does not have the possibility
of buffer overflow. However, I have not tried to fix that. For that
matter, I see other possibilities for buffer overflow in gram.y, such
as in decl_cursor_arglist. Buffer overflow of this sort is not good,
as it means that anybody who is permitted to create functions can
completely break security.

Ian

Index: gram.y
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.20
diff -u -p -r1.20 gram.y
--- gram.y	2001/05/31 17:15:40	1.20
+++ gram.y	2001/06/06 06:35:46
@@ -385,7 +385,8 @@ decl_statement	: decl_varname decl_const
*cp2++ = '\\';
*cp2++ = *cp1++;
}
-						strcat(buf, "'");
+						*cp2++ = '\'';
+						*cp2 = '\0';
curname_def->query = strdup(buf);
new->default_val = curname_def;

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

I see this was already installed by Jan.

Here is a small patch for the cursor support which Jan recently added.
The code assumed that there would be a '\0' in buf after storing the
characters in new->refname, but it did nothing to ensure that.

I can't convince myself that this code does not have the possibility
of buffer overflow. However, I have not tried to fix that. For that
matter, I see other possibilities for buffer overflow in gram.y, such
as in decl_cursor_arglist. Buffer overflow of this sort is not good,
as it means that anybody who is permitted to create functions can
completely break security.

Ian

Index: gram.y
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.20
diff -u -p -r1.20 gram.y
--- gram.y	2001/05/31 17:15:40	1.20
+++ gram.y	2001/06/06 06:35:46
@@ -385,7 +385,8 @@ decl_statement	: decl_varname decl_const
*cp2++ = '\\';
*cp2++ = *cp1++;
}
-						strcat(buf, "'");
+						*cp2++ = '\'';
+						*cp2 = '\0';
curname_def->query = strdup(buf);
new->default_val = curname_def;

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026