Date: Thu, 11 Jul 2019 11:43:48 -0400 From: Shawn Webb <shawn.webb@hardenedbsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: Enji Cooper <yaneurabeya@gmail.com>, svn-src-head@freebsd.org, svn-src-all <svn-src-all@freebsd.org>, src-committers <src-committers@freebsd.org>, Philip Paeps <philip@freebsd.org> Subject: Re: svn commit: r349896 - head/contrib/telnet/telnet Message-ID: <20190711154348.oss6ec5ysgfsiln4@mutt-hbsd> In-Reply-To: <20190712004934.Y1991@besplex.bde.org> References: <201907102236.x6AMaFLI067550@repo.freebsd.org> <6031EBD8-84D7-46D4-A3E5-D78427D084B1@gmail.com> <20190712004934.Y1991@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--bjd2hj7ltmlg53jw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 12, 2019 at 01:30:25AM +1000, Bruce Evans wrote: > On Thu, 11 Jul 2019, Enji Cooper wrote: >=20 > > > On Jul 10, 2019, at 3:36 PM, Philip Paeps <philip@FreeBSD.org> wrote: > > >=20 > > > Author: philip > > > Date: Wed Jul 10 22:36:14 2019 > > > New Revision: 349896 > > > URL: https://svnweb.freebsd.org/changeset/base/349896 > > >=20 > > > Log: > > > telnet: fix minor style violation > > >=20 > > > While here also fix a very unlikely NULL pointer dereference. >=20 > I see no fix here. >=20 > > > Modified: head/contrib/telnet/telnet/commands.c > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > > > --- head/contrib/telnet/telnet/commands.c Wed Jul 10 22:23:59 2019 (r= 349895) > > > +++ head/contrib/telnet/telnet/commands.c Wed Jul 10 22:36:14 2019 (r= 349896) > > > @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$"); > > > #include <sys/socket.h> > > > #include <netinet/in.h> > > >=20 > > > +#include <assert.h> > > > #include <ctype.h> > > > #include <err.h> > > > #include <errno.h> > > > @@ -1654,11 +1655,13 @@ env_init(void) > > > || (strncmp((char *)ep->value, "unix:", 5) =3D=3D 0))) { > > > char hbuf[256+1]; > > > char *cp2 =3D strchr((char *)ep->value, ':'); > > > + size_t buflen; >=20 > This adds a different type of style bug (indentation via spaces instead of > tabs). >=20 > > >=20 > > > gethostname(hbuf, sizeof(hbuf)); > > > hbuf[sizeof(hbuf)-1] =3D '\0'; > > > - unsigned int buflen =3D strlen(hbuf) + strlen(cp2) += 1; > > > + buflen =3D strlen(hbuf) + strlen(cp2) + 1; > > > cp =3D (char *)malloc(sizeof(char)*buflen); > > > + assert(cp !=3D NULL); > >=20 > > This will unfortunately still segfault if assert is compiled out of the= system as a no-op (-DNDEBUG). >=20 > telnet is unlikely to be compiled with -DNDEBUG, since it didn't use asse= rt() > before and this commit doesn't change its Makefile to control its new use > of assert(). >=20 > Any it doesn't fix the error either. On must arches, it turns a nice > restartable null pointer trap into an unrestartable abort(). The program > crashes in both cases. We're getting into theory, since this particular bug isn't vulnerable to this particular issue I'm about to bring up, but it is possible to map at NULL on FreeBSD, given a sysctl has been explicitly toggled by an administrative user. I've found it's best to adhere to good defensive programming techniques, even in cases where doing so may not make immediate sense. Future developers, even the code's original author(s), may be grateful later as they make changes. Thus, even if this particular potential NULL pointer dereference isn't exploitable in any meaningful way, adherence to defensive programming practices will help both now and later. One thing I love about FreeBSD is how it strives to deliver high quality, correct code. It seems strange that more characters have been written in emails about the lack of asprintf usage than it would've taken to actually write the patch to fix it. Thanks, --=20 Shawn Webb Cofounder / Security Engineer HardenedBSD Tor-ified Signal: +1 443-546-8752 Tor+XMPP+OTR: lattera@is.a.hacker.sx GPG Key ID: 0xFF2E67A277F8E1FA GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2 --bjd2hj7ltmlg53jw Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAl0nWS8ACgkQ/y5nonf4 4foCHxAAhWZrDCMz2kCTfUsZkJ9640CThBYZOcNu4kVo3wQdjei1P/YgEL1s+Ge+ rkNlRDrq+BoID1z4iEtXAAP3T9RPX0RDb/rJh1z0HsXQwVT2gQt1ClBVoJSnir5e 138uDOckI2L8as6HMEFkqfexFIfcXj2gmTKl6WEEHJcs/tpuwu5myZ3+G8LWbjed 0igr3CF4PoUrTMlguAvr0FU9V1QYO+HN49BZTjtY2/tmAc03/q05hUxU8tZSPp/P k3bY6r1agRlUOW/vax+iIE2QQYPeeqDeHSan+tX1hLvTEJBEesf2QBIZItskDDHV uMRPUQZLnqNXIeoenzXt5mrnDhr7OqgFubkAWIC0LTL5EGpBqttQ+9/sjjGUGbZP j0x98pQRf8yZkOEt1ffP/scYNL9vn8anf6UqBV5XFuDV2KZQrpYSilnW49JjNZ7S vDNUiCGjk7IcFgRXr07qhqzkkEH+lYJxcmJB9FfyYAug3gm62tjpAP+FyzO1qBT0 JZ9C0TcnacXBn5nxG3gjkwQo8qISWlq/3YGnCrVqMYfVJ0Qqrz/DvV4NhN+a2hzv S4awe1aEKYzY6DYsUuDMfXiWe5tPFswAFFwgnEsZqhy0GpohG/UQmLyA5pKs+f8e stYcZkfWXbNXEB0LR1y/B6kbjDfpMhDnNwo4tJ9IpSFsVuvmNak= =lax/ -----END PGP SIGNATURE----- --bjd2hj7ltmlg53jw--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190711154348.oss6ec5ysgfsiln4>