On Wed, Jul 10, 2019 at 03:19:44PM -0500, Justin Hibbits wrote: > On Wed, 10 Jul 2019 15:55:48 -0400 > Shawn Webb <shawn.w...@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.
While fixing the style(9) violation above, we could still take care of the potential NULL deref at the same time. If anything, just for code correctness reasons? Thanks, -- Shawn Webb Cofounder / Security Engineer HardenedBSD Tor-ified Signal: +1 443-546-8752 Tor+XMPP+OTR: latt...@is.a.hacker.sx GPG Key ID: 0xFF2E67A277F8E1FA GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2
signature.asc
Description: PGP signature