bugs in news 2.10.1, advice to authors

[Geoffrey Collyer]

I have recently found and fixed two readnews 2.10.1 bugs that
cause core dumps on v7, in the undigestifier H command and
the function findngsize.

The undigestifier (invoked by the d command to readnews) takes an
H subcommand which called hprint with an extra argument *in the
middle of the argument list*, which eventually caused a Bus Error
in fprintf.

Diffs of digest.c follow:

201c201
< 			hprint(h, !cflag, ofp, 1);
---
> 			hprint(h, ofp, 1);
209c209
< 				hprint(h, !cflag, ofp, 0);
---
> 				hprint(h, ofp, 0);

(Lines numbers are approximate; actual line numbers may vary.)

Two days later, after I had read roughly one hundred articles,
readnews took a Memory Fault and dumped core, for the third
time in three days.  Of course, by now I was getting used to it.
I dug through the core dump, and since I had wisely not stripped
my readnews binary determined that the global long ngsize had been
trashed, causing selectn() to try to zero far, far beyond the end
of bitmap[].  This happened shortly after I had deleted the most
recent round of dead news groups from /usr/lib/news/active,
which naturally made me suspect findngsize().

This is the vile swill that used to be findngsize (in rfuncs.c):

	/*
	 * Figure out the number of the largest article in newsgroup ng,
	 * and return that value.
	 */
	long
	findngsize(ng)
	char *ng;
	{
		FILE *af;
		long s;
		char buf[100], n[100];
	
		af = xfopen(ACTIVE, "r");
		while (fgets(buf, sizeof buf, af)) {
			sscanf(buf, "%s %ld", n, &s);
			if (strcmp(n, ng) == 0) {
				fclose(af);
				return s;
			}
		}
		return 0;
	}

This foul, evil-smelling excuse for a function failed to close
ACTIVE (/usr/lib/news/active) if the newsgroup ng was not found
*and* returned utter trash (whatever was on the stack) if ACTIVE
was malformed, since it failed to check that sscanf actual read
two items from buf.  I replaced it with this function:

	long
	findngsize(ng)
	char *ng;
	{
		FILE *af;
		long s, ret = 0;
		char buf[100], n[100];
	
		af = xfopen(ACTIVE, "r");
		while (fgets(buf, sizeof buf, af) != NULL)
			if (sscanf(buf, "%s %ld", n, &s) == 2 && strcmp(n, ng) == 0) {
				ret = s;
				break;
			}
		fclose(af);
		return ret;
	}

There is a further minor bug in inews.c.
At the start of localize(), the line

	actfp = fopen(ACTIVE, "r+");

appears, and actfp is never tested to see if fopen succeeded.
This particular bug hasn't bitten me, but if inews
ever can't read and write the ACTIVE file, it will start
fprintfing on NULL, which can produce bizarre results.
fcloseing NULL produces a core dump on utcsstat (a v7 system).
There are many other such unchecked system calls and function calls
throughout B news and trying to chase them all down would be an
endless task.

System calls *do* fail, even when *you* think they shouldn't or
can't or won't.  I wish people would assume the worst when writing
programs: system calls will always fail, at the most inconvenient
times, when you are least able to deal with them, for reasons that
you can't foresee.  If you at least try to do something reasonable,
rather than just ``knowing'' that nothing can possibly go wrong,
your program won't go berserk when faced with the unusual!
All right, officer, I'll go peacefully...

Geoff Collyer, U. of Toronto