*BSD News Article 33686


Return to BSD News archive

Xref: sserve comp.os.386bsd.questions:11989 comp.os.386bsd.development:2362 comp.os.386bsd.misc:2979
Newsgroups: comp.os.386bsd.questions,comp.os.386bsd.development,comp.os.386bsd.misc
Path: sserve!newshost.anu.edu.au!harbinger.cc.monash.edu.au!msuinfo!agate!howland.reston.ans.net!EU.net!sun4nl!cs.vu.nl!kjb
From: kjb@cs.vu.nl (Kees J. Bot)
Subject: Re: Why does FreeBSD 1.1.5 say gets() is unsafe?
Message-ID: <CturD6.3C1@cs.vu.nl>
Sender: news@cs.vu.nl
Organization: Fac. Wiskunde & Informatica, VU, Amsterdam
References: <30lrf3$2ii@acmez.gatech.edu> <311m2e$o33@agate.berkeley.edu> <jmonroyCtMGq2.IC6@netcom.com> <Ctn5yy.3I0@cs.vu.nl> <31cf70$3c@Starbase.NeoSoft.COM>
Date: Mon, 1 Aug 1994 10:48:41 GMT
Lines: 78

peter@Starbase.NeoSoft.COM (Peter da Silva) writes:

>In article <Ctn5yy.3I0@cs.vu.nl>, Kees J. Bot <kjb@cs.vu.nl> wrote:
>>I don't have gets() in the C library on my system (Minix-386vm) at all.
>>Any gets(buf) call that I may find is immediately replaced by:

>>	result = fgets(buf, sizeof(buf), stdin);
>>	*strchr(buf, '\n') = 0;

>This can result in writing through the null pointer if reading from a zero
>length file and the buffer is uninitialized, or on any eof if the buffer is
>reinitialized, which will core dump on OSF/1 and generate enforcer hits on
>the Amiga.

>	if(result = fgets(buf, sizeof(buf), stdin))
>		*strchr(buf, '\n') = 0;

Oops, dumb mistake.  This is what I should have written.  The gets()
calls you find are usually in the form:

	while (gets(buf) != NULL) {
		...
	}

The strchr() call then becomes the first statement in the while loop
after replacement.

>>This makes options 3) and 4) impossible, because a NULL-dereference will
>>occur if 'buf' is overrun causing a core dump.

>Um, could you explain this statement? I honestly don't understand whether
>you were intentionally creating that null dereference or not. fgets will
>not overrun the buffer, and gets overrunning the buffer has unpredictable
>results, not simply a null dereference.

Yes, I am intentionally creating a possible NULL dereference, but only
if the original gets() call would overrun the buffer.

Assume that you are porting this large package.  Alas it contains some
gets() calls.  Now one could carefully examine each gets() call to
determine if it is reading fixed length data, or one can assume that
the package author has done it right and make the fgets() + strchr()
replacement.  If the author did not do it right then at least you get a
core dump instead of a buffer overrun.

I personally don't use gets() or fgets() ever.  Typical example of my
code attached below.  (allocate() and fatal() are left to your
imagination.)
--
	                        Kees J. Bot  (kjb@cs.vu.nl)
	              Systems Programmer, Vrije Universiteit Amsterdam
_._. .._ _   ._ ._.. ___ _. __.   _ .... .   _.. ___ _ _ . _..   ._.. .. _. .

char *read1line(char *file, FILE *fp)
/* Read one line from a file.  Result is a malloc()'d string or NULL for EOF */
{
	char *line;
	size_t idx, len;
	int c;

	line= NULL;
	idx= len= 0;
	while ((c= getc(fp)) != EOF && c != '\n') {
		if (idx == len) {
			len= 2 * idx + 20;
			line= allocate(line, len * sizeof(line[0]));
		}
		line[idx++]= c;
	}
	if (c == EOF) {
		if (ferror(fp)) fatal(file);
		return NULL;
	}
	line= allocate(line, (idx+1) * sizeof(line[0]));
	line[idx]= 0;

	return line;
}