Re: [PATCH] TUI: Fix buffer overflow in tui_expand_tabs

[Eli Zaretskii <eliz@gnu.org>]

> 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; )
@@ -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