Bug 222796

Summary: [exp-run] with gets() removed
Product: Ports & Packages Reporter: Ed Maste <emaste>
Component: Ports FrameworkAssignee: Port Management Team <portmgr>
Status: Closed FIXED    
Severity: Affects Only Me CC: cy, gerald, ports-bugs, w.schwarzenfeld
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D12298
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=228914
Bug Depends on: 231066, 238682, 238685, 238686, 238687, 238688, 238690, 238691, 238692, 241168    
Bug Blocks:    
Attachments:
Description Flags
trivial patch to remove gets
none
Templates/config.site patch to remove gets
none
gets removal commit candidate
none
updated Templates/config.site diff to remove gets none

Description Ed Maste freebsd_committer 2017-10-05 13:18:35 UTC
Created attachment 186922 [details]
trivial patch to remove gets

Discussed during Paul Vixie's vBSDcon talk. gets has been deprecated for ages and we have a runtime warning if it's called.

I would like to see if any ports fail to build if gets is removed.
Comment 1 Antoine Brodin freebsd_committer 2017-10-05 13:25:12 UTC
is the src patch complete?

the ports tree needs a patch too.
Comment 2 Baptiste Daroussin freebsd_committer 2017-10-05 13:32:06 UTC
Yes this should be enough
Comment 3 Antoine Brodin freebsd_committer 2017-10-05 13:43:26 UTC
On 10.3 amd64:

./alliance-5.0.20120515_4.log:vh_debug.c:(.text+0x4e2): warning: warning: this program uses gets(), which is unsafe.
./cint-5.18.00_7.log:G__c_stdfunc.c:(.text+0x11d9): warning: warning: this program uses gets(), which is unsafe.
./diehard-0.1.log:diehard.c:(.text+0x741): warning: warning: this program uses gets(), which is unsafe.
./estic-1.61_5.log:/wrkdirs/usr/ports/misc/estic/work/estic-1.61_5/areacode/actest.c:63: warning: warning: this program uses gets(), which is unsafe.
./gcc46-4.6.4_9,1.log:/wrkdirs/usr/ports/lang/gcc46/work/gcc-4.6.4/libssp/gets-chk.c:69: warning: warning: this program uses gets(), which is unsafe.
./gcc47-4.7.4_6,1.log:/wrkdirs/usr/ports/lang/gcc47/work/gcc-4.7.4/libssp/gets-chk.c:69: warning: warning: this program uses gets(), which is unsafe.
./gcc48-4.8.5_5.log:/wrkdirs/usr/ports/lang/gcc48/work/gcc-4.8.5/libssp/gets-chk.c:69: warning: warning: this program uses gets(), which is unsafe.
./gcc49-4.9.4_4.log:/wrkdirs/usr/ports/lang/gcc49/work/gcc-4.9.4/libssp/gets-chk.c:69: warning: warning: this program uses gets(), which is unsafe.
./gcc5-5.4.0_4.log:/wrkdirs/usr/ports/lang/gcc5/work/gcc-5.4.0/libssp/gets-chk.c:74: warning: warning: this program uses gets(), which is unsafe.
./gcc5-devel-5.4.1.s20170926.log:/wrkdirs/usr/ports/lang/gcc5-devel/work/gcc-5-20170926/libssp/gets-chk.c:74: warning: warning: this program uses gets
(), which is unsafe.
./gcc6-6.4.0_2.log:/wrkdirs/usr/ports/lang/gcc6/work/gcc-6.4.0/libssp/gets-chk.c:74: warning: warning: this program uses gets(), which is unsafe.
./gcc6-devel-6.4.1.s20170927.log:/wrkdirs/usr/ports/lang/gcc6-devel/work/gcc-6-20170927/libssp/gets-chk.c:74: warning: warning: this program uses gets
(), which is unsafe.
./gcc7-7.2.0_2.log:/wrkdirs/usr/ports/lang/gcc7/work/gcc-7.2.0/libssp/gets-chk.c:74: warning: warning: this program uses gets(), which is unsafe.
./gcc7-devel-7.2.1.s20170928.log:/wrkdirs/usr/ports/lang/gcc7-devel/work/gcc-7-20170928/libssp/gets-chk.c:74: warning: warning: this program uses gets
(), which is unsafe.
./gcc8-devel-8.0.0.s20171001.log:/wrkdirs/usr/ports/lang/gcc8-devel/work/gcc-8-20171001/libssp/gets-chk.c:74: warning: warning: this program uses gets
(), which is unsafe.
./grass6-6.4.6_10,2.log:bmf_to_cll.c:(.text.startup+0x4c): warning: warning: this program uses gets(), which is unsafe.
./ja-newosaka-1.0.log:osaka.c:(.text+0x4e): warning: warning: this program uses gets(), which is unsafe.
./ko-hpscat-jshin-1.3.1,1.log:hpscat.c:(.text+0x118): warning: warning: this program uses gets(), which is unsafe.
./libsocketcpp-1.0.7_1.log:tcptest.cpp:(.text+0x4c): warning: warning: this program uses gets(), which is unsafe.
./prng-3.0.2.log:pairs.c:(.text+0x43): warning: warning: this program uses gets(), which is unsafe.
./recombine-1.41.log:/wrkdirs/usr/ports/biology/recombine/work/recombine1.41/recombine.c:172: warning: warning: this program uses gets(), which is uns
afe.
./tcpview-1.0_6.log:hex.c:(.text+0xcc): warning: warning: this program uses gets(), which is unsafe.
./tcpview-1.0_6.log:hex.c:(.text+0xcc): warning: warning: this program uses gets(), which is unsafe.
./tcpview-1.0_6.log:hex.c:(.text+0xcc): warning: warning: this program uses gets(), which is unsafe.
./vi-vnconvert-1.0.log:vnconvert.c:(.text+0x143): warning: warning: this program uses gets(), which is unsafe.
./yagiuda-1.19.log:dynamic.c:(.text+0xb3): warning: warning: this program uses gets(), which is unsafe.
./yagiuda-1.19.log:input.c:(.text+0xd1): warning: warning: this program uses gets(), which is unsafe.
Comment 4 Ed Maste freebsd_committer 2017-10-05 13:44:04 UTC
Created attachment 186923 [details]
Templates/config.site patch to remove gets
Comment 5 Ed Maste freebsd_committer 2017-10-05 13:55:55 UTC
The gcc ones come from __gets_chk, which takes a length and is used to wrap calls to gets (presumably if the compiler can determine the length it passes it in); if we remove gets we can just disable this in libssp.

That leaves 13 ports

alliance
cint
diehard
estic
grass6
ja-newosaka
ko-hpscat-jshin
libsocketcpp
prng
recombine
tcpview
vi-vnconvert
yagiuda
Comment 6 commit-hook freebsd_committer 2017-10-05 14:03:08 UTC
A commit references this bug:

Author: bapt
Date: Thu Oct  5 14:02:48 UTC 2017
New revision: 451313
URL: https://svnweb.freebsd.org/changeset/ports/451313

Log:
  Mark as deprecated, it uses gets(3) and is not depend on by anything

  PR:		222796

Changes:
  head/net/libsocketcpp/Makefile
Comment 7 Ed Maste freebsd_committer 2017-10-06 13:06:15 UTC
Updated patch (which leaves gets in existing symbol version) is at https://reviews.freebsd.org/D12298 although for test purposes either would work. That said we don't need an actual exp-run at this point as your list based on warnings in logs fills the need.

These ports using gets() are maintained by ports@:

cad/alliance
math/diehard
japanese/newosaka
korean/hpscat
net/libsocketcpp
math/prng
biology/recombine
vietnamese/vnconvert

I'll see if I can find folks willing to work on a patch for them (I think such a patch would make a good introduction to processes used in the FreeBSD ports tree).

I've also mailed the maintainers of the five maintained ports that use gets:
lang/cint
misc/estic
databases/grass6
net/tcpview
comms/yagiuda
Comment 8 commit-hook freebsd_committer 2017-10-07 01:24:33 UTC
A commit references this bug:

Author: db
Date: Sat Oct  7 01:24:17 UTC 2017
New revision: 451423
URL: https://svnweb.freebsd.org/changeset/ports/451423

Log:
  Remove the two gets() calls and replace with fgets()
  https://reviews.freebsd.org/D12298
  Whilst here redo the patches with make makepatch

  PR:		ports/222796
  Reported by:	emaste@

Changes:
  head/comms/yagiuda/files/patch-src_gain.c
  head/comms/yagiuda/files/patch-src_genetic__algorithm__lib.c
  head/comms/yagiuda/files/patch-src_genetic_algorithm_lib.c
  head/comms/yagiuda/files/patch-src_input.c
  head/comms/yagiuda/files/patch-src_output.c
  head/comms/yagiuda/files/patch-src_random.c
  head/comms/yagiuda/files/patch-src_string.c
  head/comms/yagiuda/files/patch-src_yagi.c
Comment 9 commit-hook freebsd_committer 2017-10-09 16:59:32 UTC
A commit references this bug:

Author: cy
Date: Mon Oct  9 16:58:45 UTC 2017
New revision: 451623
URL: https://svnweb.freebsd.org/changeset/ports/451623

Log:
  Use fgets() instead of gets().

  The approach I used was to create a "poor man's" gets macro as an example.
  Though not the same as gets() it approximates gets() well enough. We might
  want to consider this approach in base.

  This is for ttps://reviews.freebsd.org/D12298.

  PR:		222796
  Requested by:	emaste

Changes:
  head/net/tcpview/Makefile
  head/net/tcpview/files/patch-hex.c
Comment 10 commit-hook freebsd_committer 2018-09-01 20:18:20 UTC
A commit references this bug:

Author: gerald
Date: Sat Sep  1 20:17:46 UTC 2018
New revision: 478722
URL: https://svnweb.freebsd.org/changeset/ports/478722

Log:
  Disable the build/use of libssp/gets-chk since FreeBSD 12 and later
  do not feature gets() any longer.

  PR:		222796, 231066
  Differential Revision:	https://reviews.freebsd.org/D12298

Changes:
  head/lang/gcc7/files/patch-gets-no-more
Comment 11 Gerald Pfeifer freebsd_committer 2018-09-01 20:24:19 UTC
I reported the situation around GCC (aka lang/gcc* in our Ports
Collection) upstream, which did not lead to a resolution in terms
of code, thus I'm moving forward to patch important lang/gcc* ports
-- initially lang/gcc7 as the default, next lang/gcc8 as the future
default, and lang/gcc9-deve -- still hoping for some fixes to come
in upstream.

Older versions, and -devel versions that are actively maintained
upstream still and feature a stable sibling I'll leave as is for the
time being; users should hardly need those, let alone in combination
with FreeBSD 12.


Personal opinion: removing gets() burns a lot of cycles for a number
of us for pretty much zero practical security benefit.
Comment 12 commit-hook freebsd_committer 2018-09-02 00:09:17 UTC
A commit references this bug:

Author: gerald
Date: Sun Sep  2 00:08:51 UTC 2018
New revision: 478752
URL: https://svnweb.freebsd.org/changeset/ports/478752

Log:
  Forward port r478722 | gerald | 2018-09-01 from lang/gcc7:

  Disable the build/use of libssp/gets-chk since FreeBSD 12 and later
  do not feature gets() any longer.

  PR:		222796, 231066
  Differential Revision:	https://reviews.freebsd.org/D12298

Changes:
  head/lang/gcc8/files/patch-gets-no-more
Comment 13 commit-hook freebsd_committer 2018-09-08 08:17:54 UTC
A commit references this bug:

Author: gerald
Date: Sat Sep  8 08:16:55 UTC 2018
New revision: 479233
URL: https://svnweb.freebsd.org/changeset/ports/479233

Log:
  Forward port r478722 | gerald | 2018-09-01 from lang/gcc7:

  Disable the build/use of libssp/gets-chk since FreeBSD 12 and later
  do not feature gets() any longer.

  (I was planning to make this part of a routine update, alas last week's
  snapshot was broken on i386-unknown-freebsd10.x and so will this week's
  still.)

  PR:		222796, 231066
  Differential Revision:	https://reviews.freebsd.org/D12298

Changes:
  head/lang/gcc9-devel/files/patch-gets-no-more
Comment 14 commit-hook freebsd_committer 2018-09-30 07:36:01 UTC
A commit references this bug:

Author: gerald
Date: Sun Sep 30 07:35:17 UTC 2018
New revision: 480939
URL: https://svnweb.freebsd.org/changeset/ports/480939

Log:
  Recommend the use of GCC 7 or later over this port, since that is now
  the default version of GCC for the Ports Collection.

  And backport r478722 | gerald | 2018-09-01 from lang/gcc7: [1]

  Disable the build/use of libssp/gets-chk since FreeBSD 12 and later
  do not feature gets() any longer.

  PR:		222796, 231066 [1]
  Differential Revision:	https://reviews.freebsd.org/D12298 [1]

Changes:
  head/lang/gcc5/Makefile
  head/lang/gcc5/files/patch-gets-no-more
Comment 15 Ed Maste freebsd_committer 2018-10-25 13:58:47 UTC
Created attachment 198620 [details]
gets removal commit candidate
Comment 16 Ed Maste freebsd_committer 2018-10-25 14:01:24 UTC
Commit candidate patch attached, I would like to request another exp-run
Comment 17 Antoine Brodin freebsd_committer 2018-10-25 14:12:07 UTC
Isn't the config.site patch incomplete?  (gl_cv_have_raw_decl_gets=yes)
Comment 18 Ed Maste freebsd_committer 2018-10-27 10:57:06 UTC
Created attachment 198688 [details]
updated Templates/config.site diff to remove gets
Comment 19 Antoine Brodin freebsd_committer 2018-10-28 13:28:40 UTC
lang/gcc6-aux probably needs similar changes as lang/gcc*

Some configure part is wrong, which breaks the build later

checking whether strstr is declared... no
checking whether getenv is declared... no
checking whether atol is declared... no
checking whether atoll is declared... no
checking whether asprintf is declared... no
checking whether sbrk is declared... no
checking whether abort is declared... no
checking whether atof is declared... no
checking whether getcwd is declared... no
checking whether getwd is declared... no
checking whether madvise is declared... no

http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/gcc6-aux-20180516,1.log
Comment 20 Antoine Brodin freebsd_committer 2018-10-29 20:08:43 UTC
New failures on amd64:

+ {"origin"=>"biology/recombine", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"cad/alliance", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"dns/dnsperf", "phase"=>"build", "errortype"=>"clang"}
+ {"origin"=>"japanese/newosaka", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"korean/hpscat", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"lang/gcc6-aux", "phase"=>"build", "errortype"=>"???"}
+ {"origin"=>"math/diehard", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"math/mingw32-libgmp", "phase"=>"build", "errortype"=>"???"}
+ {"origin"=>"math/prng", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"sysutils/grub2", "phase"=>"build", "errortype"=>"???"}
+ {"origin"=>"sysutils/grub2-bhyve", "phase"=>"build", "errortype"=>"???"}
+ {"origin"=>"vietnamese/vnconvert", "phase"=>"build", "errortype"=>"linker_error"}

Around 50 ports newly skipped, mostly due to gcc6-aux

New failure logs on amd64:

http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/recombine-1.41.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/alliance-5.0.20120515_6.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/dnsperf-2.1.0.0_1.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/ja-newosaka-1.0.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/ko-hpscat-jshin-1.3.1,1.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/gcc6-aux-20180516,1.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/diehard-0.1.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/mingw32-libgmp-6.0.0_3.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/prng-3.0.2.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/grub2-2.00_12.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/grub2-bhyve-0.40_5.log
http://package18.nyi.freebsd.org/data/112amd64PR222796-default/2018-10-27_20h58m08s/logs/errors/vi-vnconvert-1.0.log
Comment 21 Antoine Brodin freebsd_committer 2018-10-29 20:27:58 UTC
The dnsperf failure may be unrelated.
Comment 22 commit-hook freebsd_committer 2019-09-01 16:12:24 UTC
A commit references this bug:

Author: emaste
Date: Sun Sep  1 16:12:07 UTC 2019
New revision: 351659
URL: https://svnweb.freebsd.org/changeset/base/351659

Log:
  libc: remove gets

  gets is unsafe and shouldn't be used (for many years now).  Leave it in
  the existing symbol version so anything that previously linked aginst it
  still runs, but do not allow new software to link against it.

  (The compatability/legacy implementation must not be static so that
  the symbol and in particular the compat sym gets@FBSD_1.0 make it
  into libc.)

  PR:		222796 (exp-run)
  Reported by:	Paul Vixie
  Reviewed by:	allanjude, cy, eadler, gnn, jhb, kib, ngie (some earlier)
  Relnotes:	Yes
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D12298

Changes:
  head/contrib/libc++/include/cstdio
  head/contrib/netbsd-tests/lib/libc/ssp/h_gets.c
  head/gnu/lib/libssp/Makefile
  head/include/stdio.h
  head/lib/libc/stdio/fgets.3
  head/lib/libc/stdio/gets.c
  head/lib/libc/stdio/stdio.3
Comment 23 commit-hook freebsd_committer 2019-09-01 18:57:49 UTC
A commit references this bug:

Author: antoine
Date: Sun Sep  1 18:57:19 UTC 2019
New revision: 510716
URL: https://svnweb.freebsd.org/changeset/ports/510716

Log:
  gets was removed from head

  PR:		222796

Changes:
  head/Templates/config.site
Comment 24 commit-hook freebsd_committer 2019-09-02 05:01:34 UTC
A commit references this bug:

Author: cy
Date: Mon Sep  2 05:00:38 UTC 2019
New revision: 510753
URL: https://svnweb.freebsd.org/changeset/ports/510753

Log:
  Use gets_s() as a more appropriate replacement for gets() instead of
  fgets().

  PR:		222796
  MFH:		2019Q3

Changes:
  head/net/tcpview/files/patch-hex.c
Comment 25 commit-hook freebsd_committer 2019-09-02 05:01:36 UTC
A commit references this bug:

Author: cy
Date: Mon Sep  2 05:00:44 UTC 2019
New revision: 510754
URL: https://svnweb.freebsd.org/changeset/ports/510754

Log:
  Fix after gets(3) retirement using gets_s(3).

  Bump PORTREVISION as this is a security issue too.

  PR:		222796
  MFH:		2019Q3

Changes:
  head/biology/recombine/Makefile
  head/math/prng/files/patch-examples_pairs.c
Comment 26 Walter Schwarzenfeld freebsd_triage 2019-10-17 14:33:43 UTC
All depending Bugs closed, so we can close here.