From owner-svn-src-user@freebsd.org Sun Feb 7 05:14:10 2016 Return-Path: Delivered-To: svn-src-user@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 23D03AA0D6E for ; Sun, 7 Feb 2016 05:14:10 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: from mail-pa0-x234.google.com (mail-pa0-x234.google.com [IPv6:2607:f8b0:400e:c03::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E2287144A; Sun, 7 Feb 2016 05:14:09 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: by mail-pa0-x234.google.com with SMTP id cy9so56296595pac.0; Sat, 06 Feb 2016 21:14:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=eYqMtG3436/JN0BD+QSrtS902HyGRshEEprmy2EP+KE=; b=PfQGmGOP18dE9NiTObGGpMjRVnBjaHEMGVHxWt/S4GNgwy4ecfevQ33QKsry5P7Npk 9xfP2YHslHB1rj5EHa9Ld6LQ2roBh6zuf5YQMYoh+a1vXqXfGX3wSws7rZ3LDh9wzfhe 2tjZKggIcomu6Ny9P5by+gS0G11JYUhfn3TMDNh4qdkWv9SKtG+0K2UZKerU9ZOLkPEQ GiAdSko09r6ko9QXQVUuEhsNuYKhkLeGrqT7kO+mtw5XD9BfuiCAc1uVB1Uvs9b+CM+G VvloQiZnYvsbRAY72bDfIIf2EAzZ8GlJl5DI9KMMf2ss9vAmKm+Mj9WupG6l91nBOJlV bcgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:content-type:mime-version:subject:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to; bh=eYqMtG3436/JN0BD+QSrtS902HyGRshEEprmy2EP+KE=; b=beIwCkTKgP2bPeEXNSKY6K+Chr66f2k6Ed1Pcb32nTriqQPt/AuLHhYj8kB5D/rWCo SmyFKd4V+xeo7knoTCe4X+MBr28i/IFuB82/teoTdq6WXFaXNd2EiTIRaejLkQ5Gz/Tx 28HRL5GHHl0pmoQyJ0/9ZzuA7iKXyQBIUGDkIHMg/0Y2PuCe/yhUAe9Dcpv0E7if5YU8 LUIKhgrmEtn/cKNrqj3loE1uEi3RjOg7bON6xZkcKqY5tgCdNXRMJ/hJp10eB0y1+JtB syqSL1pORUKVWmFlzG31hRY+Z0P5FBm0m51HTfNviqFKdAV7sF1Jc/tYpIrMwY2yiKcy LoFw== X-Gm-Message-State: AG10YORumnXrRMyAWaI1MMrDhuOyAYEY84ScU/3r9e1Czpo4bbpxfjrzn+42NSsvrF+AdA== X-Received: by 10.67.30.201 with SMTP id kg9mr32655482pad.132.1454822049507; Sat, 06 Feb 2016 21:14:09 -0800 (PST) Received: from [192.168.20.7] (c-24-16-212-205.hsd1.wa.comcast.net. [24.16.212.205]) by smtp.gmail.com with ESMTPSA id 27sm9864635pfh.48.2016.02.06.21.14.07 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 06 Feb 2016 21:14:08 -0800 (PST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Subject: Re: svn commit: r295247 - user/ngie/bsnmp_cleanup/contrib/bsnmp/lib From: NGie Cooper In-Reply-To: <20160205234838.GB97435@stack.nl> Date: Sat, 6 Feb 2016 21:14:12 -0800 Cc: Garrett Cooper , src-committers@freebsd.org, svn-src-user@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <75BF6D6C-C696-4E25-BCAF-64016DE3C287@gmail.com> References: <201602040908.u1498buB006713@repo.freebsd.org> <20160205234838.GB97435@stack.nl> To: Jilles Tjoelker X-Mailer: Apple Mail (2.2104) X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Feb 2016 05:14:10 -0000 > On Feb 5, 2016, at 15:48, Jilles Tjoelker wrote: >=20 > On Thu, Feb 04, 2016 at 09:08:37AM +0000, Garrett Cooper wrote: >> Author: ngie >> Date: Thu Feb 4 09:08:36 2016 >> New Revision: 295247 >> URL: https://svnweb.freebsd.org/changeset/base/295247 >=20 >> Log: >> Use mkstemp(3) instead of mktemp(3) when creating temporary files to = fix >> the security pragma >=20 >> Modified: >> user/ngie/bsnmp_cleanup/contrib/bsnmp/lib/snmpclient.c >=20 >> Modified: user/ngie/bsnmp_cleanup/contrib/bsnmp/lib/snmpclient.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 >> --- user/ngie/bsnmp_cleanup/contrib/bsnmp/lib/snmpclient.c Thu Feb = 4 09:07:44 2016 (r295246) >> +++ user/ngie/bsnmp_cleanup/contrib/bsnmp/lib/snmpclient.c Thu Feb = 4 09:08:36 2016 (r295247) >> @@ -1000,7 +1000,7 @@ open_client_local(const char *path) >> snprintf(snmp_client.local_path, sizeof(snmp_client.local_path), >> "%s", SNMP_LOCAL_PATH); >>=20 >> - if (mktemp(snmp_client.local_path) =3D=3D NULL) { >> + if (mkstemp(snmp_client.local_path) =3D=3D -1) { >> seterr(&snmp_client, "%s", strerror(errno)); >> (void)close(snmp_client.fd); >> snmp_client.fd =3D -1; >=20 > This shuts up the warning, but I think it will also completely break = the > client. Since mkstemp() creates a regular file, the subsequent bind() > will fail and the client will not start. Also, the file descriptor is > leaked. >=20 > The security check has guessed correctly that mktemp() is being = misused, > though. If bind() fails because of [EEXIST], it should be retried with = a > new name to guard against a DoS. Fixing the problem this way will not > make the security check happy, though. >=20 > The problem can be fixed and the security check made happy by creating = a > temporary directory with mode 700 using mkdtemp() and binding to a = name > in there. Deletion will be a two-step process. >=20 > Looking from a higher level, the bind() call may not be needed. It is > only needed if the server calls getpeername() or passes non-NULL > pointers to accept(), and uses that information. Using the pathname is > likely unsafe since it may contain symlinks. EEXIST isn=E2=80=99t documented as a valid error with bind(2): The following errors are specific to binding addresses in the UNIX domain. [ENOTDIR] A component of the path prefix is not a directory. [ENAMETOOLONG] A component of a pathname exceeded 255 characters, or = an entire path name exceeded 1023 characters. [ENOENT] A prefix component of the path name does not exist. [ELOOP] Too many symbolic links were encountered in = translating the pathname. [EIO] An I/O error occurred while making the directory entry = or allocating the inode. [EROFS] The name would reside on a read-only file system. [EISDIR] An empty pathname was specified. Testing it out, it fails with EADDRINUSE =E2=80=94 yeah, bad choice. = I=E2=80=99ll revert the commit and mute the warning. Thanks! -Ngie $ cat ~/test_bind.c=20 #include #include #include #include #include #include #include #define LOCAL_SOCK "/tmp/my.sock" int main(void) { struct sockaddr_un n; int s; close(open(LOCAL_SOCK, O_CREAT|O_WRONLY)); s =3D socket(AF_LOCAL, SOCK_DGRAM, 0); if (s =3D=3D -1) err(1, "socket"); n.sun_family =3D AF_LOCAL; strlcpy(n.sun_path, LOCAL_SOCK, sizeof(n.sun_path)); if (bind(s, (struct sockaddr*)&n, SUN_LEN(&n)) =3D=3D -1) err(1, "bind"); unlink(LOCAL_SOCK); close(s); return (0); } $ ~/test_bind=20 test_bind: bind: Address already in use=