Subject: Re: [patch] isdnloop: several buffer overflows

From: Dan Carpenter <[email-masked]>
Date: Tue, 8 Apr 2014 14:02:08 +0300

On Tue, Apr 08, 2014 at 09:34:09AM +0000, David Laight wrote:
> From: Dan Carpenter
> > There are three buffer overflows addressed in this patch.
> ...
> > 2) In isdnloop_parse_cmd(), p points to a 6 characters into a 60
> > character buffer so we have 54 characters.  The ->eazlist[] is 11
> > characters long.  I have modified the code to return if the source
> > buffer is too long.
> ...
> > @@ -903,6 +903,8 @@ isdnloop_parse_cmd(isdnloop_card *card)
> >  	case 7:
> >  		/* 0x;EAZ */
> >  		p += 3;
> > +		if (strlen(p) >= sizeof(card->eazlist[0]))
> > +			break;
> >  		strcpy(card->eazlist[ch - 1], p);
> >  		break;
> >  	case 8:
> 
> If you've done the strlen() you might as well use memcpy().
> There are also functions that will do a bounded strlen(),
> (eg memchr()).
> 

I re-wrote the patch based on your suggestion but decided that I prefer
the original just because the diff is smaller.  This is a driver that no
one uses and it's full of bugs.  Let's not worry about optimizing the
slow paths at this point.

regards,
dan carpenter