From owner-svn-src-all@freebsd.org Wed Jul 10 20:22:23 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 DFC1E15E2D66 for ; Wed, 10 Jul 2019 20:22:22 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: from mail-qk1-x742.google.com (mail-qk1-x742.google.com [IPv6:2607:f8b0:4864:20::742]) (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 68C1A70907 for ; Wed, 10 Jul 2019 20:22:22 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: by mail-qk1-x742.google.com with SMTP id r21so3013068qke.2 for ; Wed, 10 Jul 2019 13:22:22 -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=GE3Y0oRaMIibdHwfIAHAVvfwttW47sNPZaDKmkYvTbQ=; b=kKVFv7U3pVeN2HORVOKWb88nMt+sug+qgHxPtuL7d66JeRGt2wqz8HXyrlIhSz3Bpq b/znf+LcHd4DyvtWxd4sar3W9o3XeMrO9UH4bpI5ohp0GgfUctFtqt993+ntNDRSxu1l XvYBwkY/O2Nmy8dPHYvNOUR3NVqbUOc3Ki2BDshxD+r0RxH65D3UEZuNEORjpk1LBRin utIBkoDdcSmSuLElBIJnAdReaDGzQJdhYiVgBmWKdP+bR9SDCYWNc1g264nK+n6pN7do 280uDvmrb1odCYW5rSgT7zPRczY+Vm7Rzvq7ljPHFcLwdkKE639nWaMtgjNFn0BFh2cz RMyg== 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=GE3Y0oRaMIibdHwfIAHAVvfwttW47sNPZaDKmkYvTbQ=; b=E54hHb5hu4GSPUz5eZoco+8T/KJ1uoe/LPrJGmIeWvKEivU8Mv3kcgmA3Fh2S8asRC H5tKbyQfzEyf94G0v476B1UqNunP1YeGk6Kwi07Jn60XJFPEURNwXhVKcdXwqfmVcROC KD5c/XxD927UnpiNinC81CrwCX4dUMZGkBNzHDDmL/DgXc0cLuAGXxyBP4L1In/UhxKy ssadFKmEHsjnEddPjg1MC+n8us+/nwml99Wd6KnUKG1Heq32aOLOLNQFWKdzLVNKur53 RMfwy0K/ksQ+BqtVMpeDSP3HL5Wh6aoB9rklJynsUHf0QzPbe2SZ000Nbuxv0jFZdN98 x80A== X-Gm-Message-State: APjAAAUludoJvaaDCZDCV1Nbrn+Rrth3smo4e6sKxmm7lYPZEMgofGQN AmJh8iQsoXLVke4Avcknw5i9wA== X-Google-Smtp-Source: APXvYqx8TuUO6yXvDkRWhJ8doD+5gS7UGZnDNZeQEhDd3EBjRPT9DKxGiVpSijeoKGj93q1GwkCbHQ== X-Received: by 2002:a37:7a84:: with SMTP id v126mr24105370qkc.439.1562790141051; Wed, 10 Jul 2019 13:22:21 -0700 (PDT) Received: from mutt-hbsd ([151.196.118.239]) by smtp.gmail.com with ESMTPSA id b7sm1404820qtt.38.2019.07.10.13.22.19 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 10 Jul 2019 13:22:20 -0700 (PDT) Date: Wed, 10 Jul 2019 16:22:18 -0400 From: Shawn Webb To: Justin Hibbits Cc: Philip Paeps , 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: <20190710202218.yc3lcd6tsql3zkyr@mutt-hbsd> References: <201907101742.x6AHg4os016752@repo.freebsd.org> <20190710195548.kdftfemj3icarcxo@mutt-hbsd> <20190710151944.0fd94ec3@titan.knownspace> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="amaqwuyn6xpvcs6j" Content-Disposition: inline In-Reply-To: <20190710151944.0fd94ec3@titan.knownspace> 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: 68C1A70907 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.98 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.984,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: Wed, 10 Jul 2019 20:22:23 -0000 --amaqwuyn6xpvcs6j Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 10, 2019 at 03:19:44PM -0500, Justin Hibbits wrote: > On Wed, 10 Jul 2019 15:55:48 -0400 > Shawn Webb wrote: >=20 > > 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 > > >=20 > > > Log: > > > telnet: fix a couple of snprintf() buffer overflows > > > =20 > > > Obtained from: Juniper Networks > > > MFC after: 1 week > > >=20 > > > Modified: > > > head/contrib/telnet/telnet/commands.c > > > head/contrib/telnet/telnet/telnet.c > > > head/contrib/telnet/telnet/utilities.c > > >=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 > > > 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 =3D strchr((char *)ep->value, ':'); > > > =20 > > > - gethostname(hbuf, 256); > > > - hbuf[256] =3D '\0'; > > > - cp =3D (char *)malloc(strlen(hbuf) + strlen(cp2) + > > > 1); > > > - sprintf((char *)cp, "%s%s", hbuf, cp2); > > > + gethostname(hbuf, sizeof(hbuf)); > > > + hbuf[sizeof(hbuf)-1] =3D '\0'; > > > + unsigned int buflen =3D strlen(hbuf) + strlen(cp2) + > > > 1; =20 > >=20 > > buflen should be defined with the rest of the variables in the code > > block above this one. >=20 > Agreed. >=20 > >=20 > > > + cp =3D (char *)malloc(sizeof(char)*buflen); =20 > >=20 > > Lack of NULL check here leads to > >=20 > > > + snprintf((char *)cp, buflen, "%s%s", hbuf, cp2); =20 > >=20 > > potential NULL pointer deref here. >=20 > 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. >=20 > 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, --=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 --amaqwuyn6xpvcs6j Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAl0mSPUACgkQ/y5nonf4 4frvKBAAivH1YeBwZUNG74MEdAj1APyfWpZnoJ//ReAdq/KhULPB3r9KSx2kjj3K GOkigEDKoRYNgIEVAhKBG6oyI3BBdnJJfHdF4Kl+OfmB/ORuGuUQOjWA7ddcFYRH +09PsOTnKckD2NpthTf1/CDzLtmYERIl7v1b9okUb9s2df5jYZGFwBMoKcccA8fP GwZaqmOuc1tc3fALVRqi3Xd0jV4750g+pkE36ggfMxgo1srlBCc1L9jiNqOwYjiK 7a3ccsfqcJoG/hGqjcGPUzR2xtTYCZ8crMTXebhtXYq+qH1Djx9lZX14LQXroQLO EQSL53KdfP7ZzH/M+Zyp5p2xHX8MnHiuGmdr0smYpT9m6db9WDN0/tDfSN8/xdys qmWWHR7CjWIxWitwTPz2VMFRrf08i3f5PYyxn64IhUUE5tAHdThMtn0OZTrXVKRk WFFw994IX7zOucJogiJrhfeyhX+fKe1JjkIyGDigzZJwFxIfslm47YJzcmZMn124 z/WG2iO17rbtYaaCvRRJnEeUMmCHhKJSqbKw0r3j4ehpPEvjZqjLS1EKL9Hw9BoV vM2unh2GM/g9ygREf9dwKSBk5BxSHLb88g9dDgR9eb7ZAaTODWpH1Z0L78tm9uNf pdm2ag7zNkfjOvN906TV83OHGL4cH8dtnqNQU5QUTcrFBljIQOY= =WE9C -----END PGP SIGNATURE----- --amaqwuyn6xpvcs6j--