Bug 246050

Summary: Buffer overflows in fortune's strfile, unstr and randstr
Product: Base System Reporter: Shlomi Fish <shlomif>
Component: binAssignee: Mark Johnston <markj>
Status: Closed FIXED    
Severity: Affects Only Me CC: akos.somfai, emaste, markj
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
proposed patch for fortune and strfile none

Description Shlomi Fish 2020-04-30 11:26:31 UTC 
Hi all!

There are some bufferoverflows at https://svnweb.freebsd.org/base/head/usr.bin/fortune/strfile/strfile.c?revision=316500&view=markup#l299 if *argv is long enough.

Here is a fix for fortune-mod:

https://svnweb.mageia.org/packages/updates/7/fortune-mod/current/SOURCES/fortune-mod--security-buffer-overflows-w-tests.patch?view=markup&pathrev=1573463

When refactoring fortune-mod, which started as a fork of netbsd's fortune, and which I adopted, I found some buffer overflows and saw they were still present in freebsd's and netbsd's fortune. openbsd appears to have fixed them, and a netbsd developer fixed their copy after I reported it on freenode's #netbsd channel.

For more dicussion, and a reproducer:

* https://bugs.mageia.org/show_bug.cgi?id=26567

* https://github.com/shlomif/fortune-mod/commits/master
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2020-04-30 16:45:47 UTC 
A better fix would be 'svn rm usr.bin/fortune'.
Comment 2 Shlomi Fish 2020-05-03 14:29:50 UTC 
An update: in addition, an integer overflow when calculating the percentages was found:

https://github.com/shlomif/fortune-mod/commit/acd338098071bddfa1d21f87e1813727031428ea

It may or may not be exploitable.
Comment 3 Akos Somfai 2020-06-16 20:41:33 UTC 
Created attachment 215637 [details]
proposed patch for fortune and strfile

Fixing possible buffer overflow in strfile and integer overflow in fortune. Other files under fortune/ seem not affected by these problems.
Attaching patch.
Comment 4 Akos Somfai 2020-08-08 17:33:48 UTC 
Anyone interested in commiting the patch?
Comment 5 commit-hook freebsd_committer freebsd_triage 2020-08-10 17:02:39 UTC 
A commit references this bug:

Author: markj
Date: Mon Aug 10 17:01:59 UTC 2020
New revision: 364083
URL: https://svnweb.freebsd.org/changeset/base/364083

Log:
  fortune, strfile: Improve validation of command-line arguments.

  - Avoid potential overflow when parsing a percentage.
  - Avoid truncation when copying file paths.

  PR:		246050
  Submitted by:	Akos Somfai <akos.somfai@gmail.com> (original)
  MFC after:	1 week

Changes:
  head/usr.bin/fortune/fortune/fortune.c
  head/usr.bin/fortune/strfile/strfile.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2020-08-17 12:38:58 UTC 
A commit references this bug:

Author: markj
Date: Mon Aug 17 12:38:35 UTC 2020
New revision: 364298
URL: https://svnweb.freebsd.org/changeset/base/364298

Log:
  MFC r364083:
  fortune, strfile: Improve validation of command-line arguments.

  PR:	246050

Changes:
_U  stable/12/
  stable/12/usr.bin/fortune/fortune/fortune.c
  stable/12/usr.bin/fortune/strfile/strfile.c
Comment 7 Mark Johnston freebsd_committer freebsd_triage 2020-08-17 12:45:10 UTC 
Thanks for the patch.