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>