R devel - May 2008 - Bug in R 2.7 for over long lines (crasher+proposed fix!) (PR#11438)

On Sat, 2008-04-26 at 09:38 +0200, Peter Dalgaard wrote:
> bugreports at nn7.de wrote:
> > OK, I am just sending it here too as it looks like r-devel at
r-project.org
> > is not the right place:
> >   
> I think it was seen there too, just that noone got around to reply. In 
> R-bugs, there's a filing system so that it won't be completely
forgotten...
Looks like no one cares about this :(

What should I do now? I mean I pointed directly to the bug and did show
how it could be fixed....
> However, your mail seems to have gotten encoded in quoted-printable, you 
> might want to follow up with a cleaned version. (Just keep the  
> (PR#11281) in the header).
To me crashers are critical bugs... isn't really no one interested in
seeing this fixed?
> > =EF=BB=BFOn 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):
> >> =20
> >> the bug is in ./src/main/gram.c  line 3038:
> >> =20
> >>             } else { /* over-long line */
> >> fixthis --> char *LongLine =3D (char *) malloc(nc);
> >>             if(!LongLine)
> >>                 error(_("unable to allocate space for source
line %
> >>     
> > d"), xxlineno);
> >   
> >>             strncpy(LongLine, (char *)p0, nc);
> >>  bug -->    LongLine[nc] =3D '\0';
> >>             SET_STRING_ELT(source, lines++,
> >>                        mkChar2((char *)LongLine));
> >>             free(LongLine);
> >> =20
> >> note that LongLine is only nc chars long, so the
LongLine[nc]=3D'\0'
> >>     
> > might
> >   
> >> be an out of bounds write. the fix would be to do
> >> =20
> >> =EF=BB=BF            char *LongLine =3D (char *) malloc(nc+1);
> >> =20
> >> in line 3034
> >> =20
> >> 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 =3D=3D '\n' || p =3D=3D end - 1) {
> >             nc =3D p - p0;
> >             if (*p !=3D '\n')
> >             nc++;
> >             if (nc <=3D MAXLINESIZE) {
> >             strncpy((char *)SourceLine, (char *)p0, nc);
> > bug2 -->    SourceLine[nc] =3D '\0';
> >             SET_STRING_ELT(source, lines++,
> >                        mkChar2((char *)SourceLine));
> >             } else { /* over-long line */
> >             char *LongLine =3D (char *) malloc(nc+1);
> >             if(!LongLine)
> >                 error(_("unable to allocate space for source line
%d"),
> > xxlineno);
> > bug1 -->    strncpy(LongLine, (char *)p0, nc);
> >             LongLine[nc] =3D '\0';
> >             SET_STRING_ELT(source, lines++,
> >                        mkChar2((char *)LongLine));
> >             free(LongLine);
> >             }
> >             p0 =3D 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]=3D =E2=80=99\0=E2=80=99;
> >
> > this could even be made a makro / helper function ...
> >
> > And another update: This does fix the R+swig crasher for me (tested)!
> >
> > Soeren
Soeren