Thread: Cursor support buffer patch
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;
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