[Rd] Bug in R 2.7 for over long lines

[Soeren Sonnenburg]

On Fri, 2008-04-25 at 08:48 +0200, Soeren Sonnenburg wrote:
> While trying to fix swig & R2.7 I actually discovered that there is a
> bug in R 2.7 causing a crash (so R & swig might actually work):
> 
> the bug is in ./src/main/gram.c  line 3038:
> 
>             } else { /* over-long line */
> fixthis --> char *LongLine = (char *) malloc(nc);
>             if(!LongLine)
>                 error(_("unable to allocate space for source line %d"), xxlineno);
>             strncpy(LongLine, (char *)p0, nc);
>  bug -->    LongLine[nc] = '\0';
>             SET_STRING_ELT(source, lines++,
>                        mkChar2((char *)LongLine));
>             free(LongLine);
> 
> note that LongLine is only nc chars long, so the LongLine[nc]='\0' might
> be an out of bounds write. the fix would be to do
> 
>             char *LongLine = (char *) malloc(nc+1);
> 
> in line 3034
> 
> Please fix and thanks to dirk for the debian r-base-dbg package!

Looking at the code again there seems to be another bug above this for
the MAXLINESIZE test too:

        if (*p == '\n' || p == end - 1) {
            nc = p - p0;
            if (*p != '\n')
            nc++;
            if (nc <= MAXLINESIZE) {
            strncpy((char *)SourceLine, (char *)p0, nc);
bug2 -->    SourceLine[nc] = '\0';
            SET_STRING_ELT(source, lines++,
                       mkChar2((char *)SourceLine));
            } else { /* over-long line */
            char *LongLine = (char *) malloc(nc+1);
            if(!LongLine)
                error(_("unable to allocate space for source line %d"), xxlineno);
bug1 -->    strncpy(LongLine, (char *)p0, nc);
            LongLine[nc] = '\0';
            SET_STRING_ELT(source, lines++,
                       mkChar2((char *)LongLine));
            free(LongLine);
            }
            p0 = p + 1;
        }


So I guess the test would be for nc < MAXLINESIZE above or to change
SourceLine to have MAXLINESIZE+1 size.

Alternatively as the strncpy manpage suggests do this for all
occurrences of strncpy

           strncpy(buf, str, n);
           if (n > 0)
               buf[n - 1]= ’\0’;

this could even be made a makro / helper function ...

And another update: This does fix the R+swig crasher for me (tested)!

Soeren