Bug #702

tput calls gets()

Added by Garrett D'Amore about 8 years ago. Updated over 6 years ago.

Status:ResolvedStart date:2011-02-07
Priority:LowDue date:
Assignee:Gary Mills% Done:

100%

Category:cmd - userland programs
Target version:-
Difficulty:Bite-size Tags:

Description

tput calls gets() but should use fgets().

176 
177 while (gets(buff) != NULL) {
178 /* read standard input line; skip over empty lines */

gets is prone to buffer overruns, so it would be good to use fgets() as a matter of course.

History

#1 Updated by Roland Mainz about 8 years ago

  • Assignee set to Roland Mainz

Taking bug...

#2 Updated by Garrett D'Amore about 8 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

Fixed in:

changeset: 13289:0a79ebc0f4b3
tag: tip
user: Roland Mainz <
date: Thu Feb 17 14:49:38 2011 -0800
description:
702 tput calls gets()
703 hashmake calls gets() but should use fgets().
Reviewed by: Dan McDonald <>
Reviewed by: Olga Kryzhanoska <>
Approved by: Garrett D'Amore <>

#3 Updated by Garrett D'Amore about 8 years ago

  • Status changed from Resolved to New
  • % Done changed from 100 to 10

FAIL. The change has been backed out, because it failed to consider that fgets includes \n in the collected data (normally). This needs further work.

#4 Updated by Roland Mainz about 8 years ago

The root problem of the issue was that we tested (both "tput" and
"hashmake") with something like this:
$ diff -u <(/usr/bin/<prog-to-test>) <($ROOT/usr/bin/<prog-to-test>) #
This would've worked perfectly fine if we would've executed this
within a "bldenv" environment... but it looks we ran the tests
outside "bldenv".

This is quite a humilation because a simple $ set -o nounset # in the test chain would've prevented that... I've teaching that students since a decade... and now fell into the same idiot trap. Grrr... (and sorry...) ...

#5 Updated by Garrett D'Amore over 7 years ago

  • Assignee deleted (Roland Mainz)
  • Difficulty set to Bite-size
  • Tags set to needs-triage

#6 Updated by Gary Mills over 6 years ago

  • Status changed from New to Feedback
  • Assignee set to Gary Mills
  • % Done changed from 10 to 80

I have a fix that eliminates the buffer overflow that can occur in the gets() and sscanf() functions. Here's an example with the original `tput':

$ tput -S <<!
> ddddddddddddddddddddddddddddddddddddddddddddddddd                          <
> !
Memory fault(coredump)

This is with the fixed version:
$ usr/src/cmd/tput/tput -S <<!
> dddddddddddddddddddddddddddddddddddddddddddddddd                           <
> !
usr/src/cmd/tput/tput: unknown terminfo capability 'ddddddddddddddddddddddddddddddd'

#7 Updated by Garrett D'Amore over 6 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 80 to 100
  • Tags deleted (needs-triage)

Resolved in

commit d23589482994f9ec7e17f81415324eea6e26d015
Author: Gary Mills <>
Date: Wed Oct 17 06:40:09 2012 -0700

702 tput calls gets()
Reviewed by: Dan McDonald &lt;&gt;
Reviewed by: Gordon Ross &lt;&gt;
Approved by: Garrett D'Amore &lt;&gt;

Also available in: Atom