Re: [PATCH] Fix buffer overflow in config parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thomas Jarosch, 09.04.2009:
> On Thursday, 9. April 2009 01:15:17 Markus Heidelberg wrote:
> > > > diff --git a/config.c b/config.c
> > > > index b76fe4c..a9c67e8 100644
> > > > --- a/config.c
> > > > +++ b/config.c
> > > > @@ -72,7 +72,7 @@ static char *parse_value(void)
> > > >                         }
> > > >                 }
> > > >                 if (space) {
> > > > -                       if (len)
> > > > +                       if (len && len < sizeof(value)-1)
> > > >                                 value[len++] = ' ';
> > > >                         space = 0;
> >
> > Eh, or maybe better add a "continue;" here, so that only one char per
> > loop is read.
> 
> Thanks for the review.
> 
> If I understand the intention of the complete code correctly, the idea was
> to read in 1+ spaces and put -one- space in the buffer as soon as the first
> non-space character is encountered (if not inside quotes).
> 
> Adding a "continue" statement would eat up the first non-space character.

Yes, you are right, I judged to quickly.

> I guess it's ok to modify the first size check or keep to problem local and 
> check the size before putting the space in the buffer.

Keeping the problem local would mean to add this check to the end of the
"for" loop before "value[len++] = c;" where the overflow actually
happens.
OTOH this overflow only occurs within a whitespace area, so from this
POV it is local at this place.

I think the cleanest solution would be to decrease the existing check at
the top by 1, even if then the maximum length will be decreased. But
there is no hint in the docs about it anyway.

> Guess that's up to
> the maintainer which method he prefers.

Take what you think is the best solution and convince him :)

Markus

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]