Date: Thu, 11 Jul 2019 00:57:27 -0700 From: Eitan Adler <lists@eitanadler.com> To: Philip Paeps <philip@freebsd.org> Cc: src-committers <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: <CAF6rxgnD8KhoqXXqQKVtvB_dBxfogr9TZPEUnoyFPA_FVDjfOQ@mail.gmail.com> In-Reply-To: <201907101742.x6AHg4os016752@repo.freebsd.org> References: <201907101742.x6AHg4os016752@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 10 Jul 2019 at 10:42, Philip Paeps <philip@freebsd.org> 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; > Shouldn't this be `size_t` ? > + cp = (char *)malloc(sizeof(char)*buflen); > sizeof(char) is always 1 and is odd to see. Don't cast the return value of `malloc`. > + snprintf((char *)cp, buflen, "%s%s", hbuf, cp2); > Is the cast of `cp` here required? Also couldn't this be replaced with asprintf? free(ep->value); > ep->value = (unsigned char *)cp; > } > > Modified: head/contrib/telnet/telnet/telnet.c > > ============================================================================== > --- head/contrib/telnet/telnet/telnet.c Wed Jul 10 17:21:59 2019 > (r349889) > +++ head/contrib/telnet/telnet/telnet.c Wed Jul 10 17:42:04 2019 > (r349890) > @@ -785,7 +785,7 @@ suboption(void) > name = gettermname(); > len = strlen(name) + 4 + 2; > if (len < NETROOM()) { > - sprintf(temp, "%c%c%c%c%s%c%c", IAC, SB, TELOPT_TTYPE, > + snprintf(temp, sizeof(temp), "%c%c%c%c%s%c%c", IAC, SB, > TELOPT_TTYPE, > TELQUAL_IS, name, IAC, SE); > ring_supply_data(&netoring, temp, len); > printsub('>', &temp[2], len-2); > @@ -807,7 +807,7 @@ suboption(void) > > TerminalSpeeds(&ispeed, &ospeed); > > - sprintf((char *)temp, "%c%c%c%c%ld,%ld%c%c", IAC, SB, > TELOPT_TSPEED, > + snprintf((char *)temp, sizeof(temp), "%c%c%c%c%ld,%ld%c%c", > IAC, SB, TELOPT_TSPEED, > TELQUAL_IS, ospeed, ispeed, IAC, SE); > len = strlen((char *)temp+4) + 4; /* temp[3] is 0 ... */ > > > Modified: head/contrib/telnet/telnet/utilities.c > > ============================================================================== > --- head/contrib/telnet/telnet/utilities.c Wed Jul 10 17:21:59 2019 > (r349889) > +++ head/contrib/telnet/telnet/utilities.c Wed Jul 10 17:42:04 2019 > (r349890) > @@ -629,7 +629,7 @@ printsub(char direction, unsigned char *pointer, int l > } > { > char tbuf[64]; > - sprintf(tbuf, "%s%s%s%s%s", > + snprintf(tbuf, sizeof(tbuf), "%s%s%s%s%s", > pointer[2]&MODE_EDIT ? "|EDIT" : "", > pointer[2]&MODE_TRAPSIG ? "|TRAPSIG" : "", > pointer[2]&MODE_SOFT_TAB ? "|SOFT_TAB" : "", > > -- Eitan Adler
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF6rxgnD8KhoqXXqQKVtvB_dBxfogr9TZPEUnoyFPA_FVDjfOQ>