From owner-svn-src-all@freebsd.org Thu Jul 11 15:43:51 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 690D915D6F75 for ; Thu, 11 Jul 2019 15:43:51 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: from mail-vs1-xe42.google.com (mail-vs1-xe42.google.com [IPv6:2607:f8b0:4864:20::e42]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id F14E284E65 for ; Thu, 11 Jul 2019 15:43:50 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: by mail-vs1-xe42.google.com with SMTP id v6so4547071vsq.4 for ; Thu, 11 Jul 2019 08:43:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hardenedbsd.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=bsVkA+MXjLWNwM1jvGg2JecalO0cc3ZNjBjRRAMgihM=; b=MXJv/uXACmIsNo9zMGF4+DvnLjKe16uSB2OIN4AXLBMso0wSxURKy/Pg6a8JCJ95/r RflOCfeUOI9qyzoWAJvrZsxKZo04uFI/u+tOBoAYJz1NDYq0pf2nHjO+5//tjYfAr3ef uwt5VnkJt+GIu78+W1t2FMw6BOWKBVRpQZf50hfHlnrySnTQ36AdhR5jhUHNx7SSNRTo 796rE5aD9subf9BlD9xP/AQg16lnCCAhxW0KXaVJn6uuG2AAgx9pwOYvtfg4vf0wWUFD VMYqxmO5T0AWOurgsdKwJOJDP/fSmtkhDpaB/hPgD/aRDNahEmrhmuxUurG0qEuolJPL DC+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=bsVkA+MXjLWNwM1jvGg2JecalO0cc3ZNjBjRRAMgihM=; b=d45DKYKdzts/B/gd8IFqY8H287LZhEW/QRKZiulRWuI7rMmaDK4cVoPT8RZVs4Npn6 6kYWnGEHwl6tXW7iYs0AQVCmxAlBVdlDU4E/yGEOsGJFNF2VspCJvLAAha/0zVNevmVn L2h7SrsPgNIjCABUU2/MS+tPGaMEmZld8LYd1ExC9vSEKugN+mplPhVY9fw7KmRlSuMG Zf/n3BVXIRvXwClmfmnhqkI7fw4iaCfvmxI0o4IixxdKz/zD4q/p6DuT/KV2ozdNoGVK 1dsTHUCto/EaUT+S9aOYYw/VyJbhKnHBIlk6FbFfxxloUlAMQKMho3CxVVDCdW3dXTWC gDVg== X-Gm-Message-State: APjAAAXB7Wu0E9wtXw5s1uo+HfULcsLvmC2oeQZl7EKWyMdjOt3DQzdC MEIlnJqLlD4xzZsLalzoZiIp/Q== X-Google-Smtp-Source: APXvYqzDZf7vJ2I5q7eOgY04+qmhM5TlvXqOc4S0PeKHDC/4iANf/2ohkZIRx7cN1JDwqTcF5AGsIg== X-Received: by 2002:a67:1e44:: with SMTP id e65mr5379085vse.45.1562859830168; Thu, 11 Jul 2019 08:43:50 -0700 (PDT) Received: from mutt-hbsd ([63.88.83.108]) by smtp.gmail.com with ESMTPSA id s67sm2990340vkb.30.2019.07.11.08.43.49 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Thu, 11 Jul 2019 08:43:49 -0700 (PDT) Date: Thu, 11 Jul 2019 11:43:48 -0400 From: Shawn Webb To: Bruce Evans Cc: Enji Cooper , svn-src-head@freebsd.org, svn-src-all , src-committers , Philip Paeps Subject: Re: svn commit: r349896 - head/contrib/telnet/telnet Message-ID: <20190711154348.oss6ec5ysgfsiln4@mutt-hbsd> References: <201907102236.x6AMaFLI067550@repo.freebsd.org> <6031EBD8-84D7-46D4-A3E5-D78427D084B1@gmail.com> <20190712004934.Y1991@besplex.bde.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bjd2hj7ltmlg53jw" Content-Disposition: inline In-Reply-To: <20190712004934.Y1991@besplex.bde.org> X-Operating-System: FreeBSD mutt-hbsd 13.0-CURRENT-HBSD FreeBSD 13.0-CURRENT-HBSD X-PGP-Key: http://pgp.mit.edu/pks/lookup?op=vindex&search=0xFF2E67A277F8E1FA User-Agent: NeoMutt/20180716 X-Rspamd-Queue-Id: F14E284E65 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.99 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.99)[-0.995,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Jul 2019 15:43:51 -0000 --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 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 > > > #include > > >=20 > > > +#include > > > #include > > > #include > > > #include > > > @@ -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--