svn commit: r349890 - head/contrib/telnet/telnet

[Justin Hibbits]

On Wed, 10 Jul 2019 15:55:48 -0400
Shawn Webb <shawn.webb at hardenedbsd.org> wrote:

> On Wed, Jul 10, 2019 at 05:42:04PM +0000, Philip Paeps wrote:
> > Author: philip
> > Date: Wed Jul 10 17:42:04 2019
> > New Revision: 349890
> > URL: https://svnweb.freebsd.org/changeset/base/349890
> > 
> > Log:
> >   telnet: fix a couple of snprintf() buffer overflows
> >   
> >   Obtained from:	Juniper Networks
> >   MFC after:	1 week
> > 
> > Modified:
> >   head/contrib/telnet/telnet/commands.c
> >   head/contrib/telnet/telnet/telnet.c
> >   head/contrib/telnet/telnet/utilities.c
> > 
> > Modified: head/contrib/telnet/telnet/commands.c
> > ==============================================================================
> > --- head/contrib/telnet/telnet/commands.c	Wed Jul 10
> > 17:21:59 2019	(r349889) +++
> > head/contrib/telnet/telnet/commands.c	Wed Jul 10 17:42:04
> > 2019	(r349890) @@ -1655,10 +1655,11 @@ env_init(void) char
> > hbuf[256+1]; char *cp2 = strchr((char *)ep->value, ':');
> >  
> > -		gethostname(hbuf, 256);
> > -		hbuf[256] = '\0';
> > -		cp = (char *)malloc(strlen(hbuf) + strlen(cp2) +
> > 1);
> > -		sprintf((char *)cp, "%s%s", hbuf, cp2);
> > +		gethostname(hbuf, sizeof(hbuf));
> > +		hbuf[sizeof(hbuf)-1] = '\0';
> > +                unsigned int buflen = strlen(hbuf) + strlen(cp2) +
> > 1;  
> 
> buflen should be defined with the rest of the variables in the code
> block above this one.

Agreed.

> 
> > +		cp = (char *)malloc(sizeof(char)*buflen);  
> 
> Lack of NULL check here leads to
> 
> > +		snprintf((char *)cp, buflen, "%s%s", hbuf, cp2);  
> 
> potential NULL pointer deref here.

I'm not sure if this is actually a problem.  env_init() is called
exactly once, at the beginning of main(), and the environment size is
fully constrained by the OS.

That said, this file it the only one in this component that does not
check the return value of malloc().  All other uses, outside of this
file, check and error.

> 
> Thanks,
> 

- Justin