Subject: Re: bin/34658: [dM] identd truncates queries to first segment
To: None <peter@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,>
From: der Mouse <mouse@Rodents.Montreal.QC.CA>
List: netbsd-bugs
Date: 09/29/2006 15:15:04
The following reply was made to PR bin/34658; it has been noted by GNATS.

From: der Mouse <mouse@Rodents.Montreal.QC.CA>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/34658: [dM] identd truncates queries to first segment
Date: Fri, 29 Sep 2006 11:07:03 -0400 (EDT)

 >> +	while (1) {
 >> +		if ((n = recv(fd, &buf[qlen], sizeof(buf)-qlen, 0)) < 0) {
 >> +			fatal("recv");
 >> +		} else if (n == 0) {
 > [...]
 >> +		}
 > [...]
 >> +		qlen += n;
 >> +		if ( (qlen >= 2) &&
 >> +		     (buf[qlen-2] == '\r') &&
 >> +		     (buf[qlen-1] == '\n') )
 >> +			break;
 >>  	}
 
 >  This looks like asking for a buffer overflow to me.  There's no
 >  protection that prevents qlen to exceed "sizeof buf".
 
 For that to happen, recv() would have to return a value greater than
 its third argument.  I was under the impression this was "impossible".
 It'd be easy enough to add "if (qlen+n > sizeof(buf)) {...}" as a guard
 if it's not impossible.
 
 Come to think of it, there should be a test to see if buf is full, too.
 
 +	while (1) {
 +		if (qlen >= sizeof(buf)) {
 +			/* buf filled - ridiculously large query */
 +			exit(0);
 +		}
 +		if ((n = recv(fd, &buf[qlen], sizeof(buf)-qlen, 0)) < 0) {
 
 /~\ The ASCII				der Mouse
 \ / Ribbon Campaign
  X  Against HTML	       mouse@rodents.montreal.qc.ca
 / \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B