[PATCH] TUI: Fix buffer overflow in tui_expand_tabs
Doug Evans
dje@google.com
Fri Mar 20 16:24:00 GMT 2015
Eli Zaretskii writes:
> > Date: Thu, 19 Mar 2015 15:57:58 -0700
> > From: Doug Evans <dje@google.com>
> > Cc: gdb-patches <gdb-patches@sourceware.org>, Eli Zaretskii <eliz@gnu.org>
> >
> > > +2015-03-17 Anton Blanchard <anton@samba.org>
> > > +
> > > + * tui/tui-io.c (tui_expand_tabs): Zero col before reusing.
> > > +
> > > 2015-03-16 John Baldwin <jhb@FreeBSD.org>
> > >
> > > * fbsd-tdep.c (fbsd_make_corefile_notes): Fetch all target registers
> > > diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> > > index a8af9b6..02ae17d 100644
> > > --- a/gdb/tui/tui-io.c
> > > +++ b/gdb/tui/tui-io.c
> > > @@ -690,7 +690,7 @@ tui_expand_tabs (const char *string, int col)
> > > ret = q = xmalloc (strlen (string) + n_adjust + 1);
> > >
> > > /* 2. Copy the original string while replacing TABs with spaces. */
> > > - for (s = string; s; )
> > > + for (col = 0, s = string; s; )
> > > {
> > > char *s1 = strpbrk (s, "\t");
> > > if (s1)
> >
> > Hi.
> >
> > col needs to be reset to its original value on function entry, right?
> > I suggest changing the code so that col is left unmodified,
> > and use a new variable to track the advance of col in both loops.
>
> Sorry about the bug. Does the below look correct?
>
> --- gdb/tui/tui-io.c~ 2015-02-20 19:11:44.000000000 +0200
> +++ gdb/tui/tui-io.c 2015-03-20 10:01:11.289375000 +0200
> @@ -761,6 +761,7 @@ tui_expand_tabs (const char *string, int
> int n_adjust;
> const char *s;
> char *ret, *q;
> + int nc = col;
>
> /* 1. How many additional characters do we need? */
> for (n_adjust = 0, s = string; s; )
Nit. To my eyes the following would be more readable.
- int n_adjust;
+ int nc, n_adjust;
const char *s;
char *ret, *q;
/* 1. How many additional characters do we need? */
- for (n_adjust = 0, s = string; s; )
+ for (nc = col, n_adjust = 0, s = string; s; )
> @@ -768,10 +769,10 @@ tui_expand_tabs (const char *string, int
> s = strpbrk (s, "\t");
> if (s)
> {
> - col += (s - string) + n_adjust;
> + nc += (s - string) + n_adjust;
> /* Adjustment for the next tab stop, minus one for the TAB
> we replace with spaces. */
> - n_adjust += 8 - (col % 8) - 1;
> + n_adjust += 8 - (nc % 8) - 1;
> s++;
> }
> }
> @@ -780,7 +781,7 @@ tui_expand_tabs (const char *string, int
> ret = q = xmalloc (strlen (string) + n_adjust + 1);
>
> /* 2. Copy the original string while replacing TABs with spaces. */
> - for (s = string; s; )
> + for (s = string, nc = col; s; )
> {
> char *s1 = strpbrk (s, "\t");
> if (s1)
> @@ -789,12 +790,12 @@ tui_expand_tabs (const char *string, int
> {
> strncpy (q, s, s1 - s);
> q += s1 - s;
> - col += s1 - s;
> + nc += s1 - s;
> }
> do {
> *q++ = ' ';
> - col++;
> - } while ((col % 8) != 0);
> + nc++;
> + } while ((nc % 8) != 0);
> s1++;
> }
> else
Ok with that change.
Thanks!
More information about the Gdb-patches
mailing list