BUG+FIX: Buffer overrun in Log keyword expansion causing segfault

[Stewart Brodie]

We have recently come across a history file in our repository which
causes CVS to suffer a segmentation fault (Intel Solaris 2.7, CVS
1.11.1p1) when it attempts to substitute the Log keywords.

After further investigation with gdb, this crash is occurring inside
the C library's malloc routine.  I believe this to be due to a heap
corruption caused by an earlier buffer overrun during the substitution
of the Log keyword.

The line contents to the left of the $Log marker is copied onto
subsequent lines in order to preserve the indentation in the source
code.  The number of times this text is to be inserted is multiplied by
the number of newlines in the log message.  However, if the prefix is
long enough and if the log message does NOT end in a newline character,
then this count is wrong and the xrealloc at rcs.c line 3753 fails to
allocate enough space for the inserted text.

Manually editing the file inside the repository and inserting a newline
character at the end of the log message stops the segfaults.  The
occurrence in the affected file is:

*  History     :  $Log: cmlog.c $

The section of the history file containing the log message is:

1.6
log
@Altered so that the sysLog local is deleted includeing the stored
NVRam (10 last entries) @
text

The patch I include below will increment the newline count if the message
did not end in \n, and will add a \n in the substituted output.

It is unclear to me whether this, particularly the second part, is
desirable - however the prefix appears concatenated to the last line of
the log message without the insertion of the newline.

An alternative simpler patch might just be to initialise cnl to 1
insert of to 0 at line 3749.

I grant permission to distribute this patch under the terms of the GNU
Public License.


diff -c -r orig-cvs-1.11.1p1/src/ChangeLog cvs-1.11.1p1/src/ChangeLog
*** orig-cvs-1.11.1p1/src/ChangeLog     Fri Apr 27 20:57:23 2001
--- cvs-1.11.1p1/src/ChangeLog  Wed Sep 19 13:14:26 2001
***************
*** 1,3 ****
--- 1,8 ----
+ 2001-09-19  Stewart Brodie  <stewart@eh.org>
+ 
+       * rcs.c (expand_keywords): Buffer overrun fixed in substitution
+   of Log keyword.
+ 
  2001-04-27  Larry Jones  <larry.jones@sdrc.com>
  
        * main.c (lookup_command_attribute): Lookup specified command, not
diff -c -r orig-cvs-1.11.1p1/src/rcs.c cvs-1.11.1p1/src/rcs.c
*** orig-cvs-1.11.1p1/src/rcs.c Tue Apr 24 19:14:53 2001
--- cvs-1.11.1p1/src/rcs.c      Wed Sep 19 13:00:00 2001
***************
*** 3752,3757 ****
--- 3752,3763 ----
                if (*snl == '\n')
                    ++cnl;
  
+           /* If the log message did not end in a newline, increment
+              the newline count so we have space for the extra leader.
+              Failure to do so results in a buffer overrun */
+           if (*log != '\0' && snl[-1] != '\n')
+               ++cnl;
+ 
            date = printable_date (ver->date);
            sub = xrealloc (sub,
                            (sublen
***************
*** 3799,3804 ****
--- 3805,3815 ----
                        ++slnl;
                    memcpy (sub + sublen, sl, slnl - sl);
                    sublen += slnl - sl;
+                   if (slnl == logend && slnl[-1] != '\n')
+                   {
+                       sub[sublen] = '\n';
+                       ++sublen;
+                   }
                    sl = slnl;
                }
            }