Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Jul 2019 15:19:44 -0500
From:      Justin Hibbits <chmeeedalf@gmail.com>
To:        Shawn Webb <shawn.webb@hardenedbsd.org>
Cc:        Philip Paeps <philip@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r349890 - head/contrib/telnet/telnet
Message-ID:  <20190710151944.0fd94ec3@titan.knownspace>
In-Reply-To: <20190710195548.kdftfemj3icarcxo@mutt-hbsd>
References:  <201907101742.x6AHg4os016752@repo.freebsd.org> <20190710195548.kdftfemj3icarcxo@mutt-hbsd>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 10 Jul 2019 15:55:48 -0400
Shawn Webb <shawn.webb@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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190710151944.0fd94ec3>