Bug 30968

On Mon, Oct 01, 2001 at 03:25:31PM -0500, Sean Kelly wrote:
> 
> >Number:         30968
> >Category:       bin
> >Synopsis:       whois client bug w/ .biz
> >Responsible:    freebsd-bugs
> >State:          open
> >Originator:     Sean Kelly
> >Release:        FreeBSD 4.4-STABLE i386
> >Organization:
> >Environment:
> System: FreeBSD edgemaster.zombie.org 4.4-STABLE FreeBSD 4.4-STABLE #1: Sat Sep 29 22:12:48 CDT 2001 root@edgemaster.zombie.org:/usr/obj/usr/src/sys/EDGEMASTER i386
> 	Multiple machines, /usr/src/usr.bin/whois/whois.c version 1.15.2.4
> >Description:
> 	The whois client mangles output when doing specific searches and
> 	presented with specific results, specifically with results lacking
> 	a final CR.
> >How-To-Repeat:
> 	First, do 'whois haha.biz'.  Notice the odd output?
> 	Now, do 'whois haha.biz|cat'.  Notice the different output?

These are both due to the fact that, as you noticed, the final CR
is missing.  The whois(1) code assumes incorrectly that fgetln(3)
will always return a isspace(3)-terminated string.  This is not
the case, as noted in a prominent warning on the fgetln(3) manual page.
As a result, an out-of-bound string access is made.  For some reason,
when the output is sent to a terminal, that out-of-bound access reads
the previously read contents of /etc/services (whois(1) needs that
to determine which port the whois/tcp service is on).

Can you try the attached patch?

G'luck,
Peter

-- 
This sentence no verb.

Index: src/usr.bin/whois/whois.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/whois/whois.c,v
retrieving revision 1.15.2.4
diff -u -r1.15.2.4 whois.c
--- src/usr.bin/whois/whois.c	2001/08/02 02:21:24	1.15.2.4
+++ src/usr.bin/whois/whois.c	2001/10/02 16:15:22
@@ -51,6 +51,7 @@
 #include <arpa/inet.h>
 #include <ctype.h>
 #include <err.h>
+#include <errno.h>
 #include <netdb.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -267,6 +268,17 @@
 	nhost = NULL;
 	nomatch = 0;
 	while ((buf = fgetln(sfi, &len)) != NULL) {
+		if ((len == 0) || !isspace(buf[len - 1])) {
+			char *newbuf;
+
+			newbuf = realloc(buf, len + 1);
+			if (newbuf == NULL) {
+				errno = ENOMEM;
+				err(1, "reallocating");
+			}
+			newbuf[len] = '\0';
+			buf = newbuf;
+		}
 		while (len && isspace(buf[len - 1]))
 			buf[--len] = '\0';

Comment 2 Peter Pentchev 2001-10-02 12:22:44 UTC

State Changed
From-To: open->feedback

I suggested a patch in the audit-trail.

On Tue, Oct 02, 2001 at 11:47:49AM -0400, Garrett Wollman wrote:
> <<On Tue, 2 Oct 2001 04:20:02 -0700 (PDT), Peter Pentchev <roam@ringlet.net> said:
> 
> >   	while ((buf = fgetln(sfi, &len)) != NULL) {
> >  +			newbuf = realloc(buf, len + 1);
> 
> You can't do this.  The buffer fgetln() returns belongs to stdio (it
> may be a pointer into the FILE's buffer).

Oh.. oops! :)  Guess I didn't read the manpage too carefully, either..

Thanks, here's an updated patch.

G'luck,
Peter

-- 
Thit sentence is not self-referential because "thit" is not a word.

Index: src/usr.bin/whois/whois.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/whois/whois.c,v
retrieving revision 1.15.2.4
diff -u -r1.15.2.4 whois.c
--- src/usr.bin/whois/whois.c	2001/08/02 02:21:24	1.15.2.4
+++ src/usr.bin/whois/whois.c	2001/10/02 21:42:46
@@ -51,6 +51,7 @@
 #include <arpa/inet.h>
 #include <ctype.h>
 #include <err.h>
+#include <errno.h>
 #include <netdb.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -243,7 +244,7 @@
 {
 	FILE *sfi, *sfo;
 	struct addrinfo *res2;
-	char *buf, *nhost, *p;
+	char *abuf, *buf, *nhost, *p;
 	int i, nomatch, s;
 	size_t len;
 
@@ -267,6 +268,16 @@
 	nhost = NULL;
 	nomatch = 0;
 	while ((buf = fgetln(sfi, &len)) != NULL) {
+		abuf = NULL;
+		if ((len == 0) || !isspace(buf[len - 1])) {
+			abuf = calloc(1, len + 1);
+			if (abuf == NULL) {
+				errno = ENOMEM;
+				err(1, "reallocating");
+			}
+			memcpy(abuf, buf, len);
+			buf = abuf;
+		}
 		while (len && isspace(buf[len - 1]))
 			buf[--len] = '\0';
 
@@ -296,6 +307,7 @@
 				nomatch = 1;
 		}
 		printf("%s\n", buf);
+		free(abuf);
 	}
 
 	/* Do second lookup as needed. */

On Tue, Oct 02, 2001 at 02:06:32PM +0300, Peter Pentchev wrote:
...
> Can you try the attached patch?

Worked beautifully.

-- 
Sean Kelly         | PGP KeyID: 77042C7B
smkelly@zombie.org | http://www.zombie.org

For PGP key, send e-mail with subject "send pgp key"

On Wed, Oct 03, 2001 at 01:55:53AM -0700, roam@FreeBSD.org wrote:
> Synopsis: whois client bug w/ .biz
> 
> State-Changed-From-To: feedback->analyzed
> State-Changed-By: roam
> State-Changed-When: Wed Oct 3 01:54:48 PDT 2001
> State-Changed-Why: 
> I'll commit the second patch after it has been properly reviewed.
> 
> 
> Responsible-Changed-From-To: freebsd-bugs->roam
> Responsible-Changed-By: roam
> Responsible-Changed-When: Wed Oct 3 01:54:48 PDT 2001
> Responsible-Changed-Why: 
> The originator said the patches work fine.

Oh.. and of course it would be just like me to swap the reasons
for these two changes :)

G'luck,
Peter

-- 
Thit sentence is not self-referential because "thit" is not a word.

Comment 6 Peter Pentchev 2001-10-03 09:54:48 UTC

State Changed
From-To: feedback->analyzed

I'll commit the second patch after it has been properly reviewed. 

Comment 7 Peter Pentchev 2001-10-03 09:54:48 UTC

Responsible Changed
From-To: freebsd-bugs->roam

The originator said the patches work fine.

Comment 8 Peter Pentchev 2001-10-08 12:26:28 UTC

Responsible Changed
From-To: roam->mike

Actually it is Mike Barcroft, the whois(1) maintainer, who came up 
with a better patch.

Comment 9 Mike Barcroft 2001-12-09 22:39:16 UTC

State Changed
From-To: analyzed->closed


Fixed in -CURRENT and -STABLE.