openssh unix dev - Sep 2000 - off-by-one errors in getnameinfo()

Description:

getnameinfo() (confirmed for CVS version 1.14.2.1) does this sort of
buffer size checks (these is just two of many cases):

                        if (strlen(sp->s_name) > servlen)
                                return ENI_MEMORY;
                        strcpy(serv, sp->s_name);
...

                        if (strlen(hp->h_name) > hostlen) {
                                return ENI_MEMORY;
                        }
                        strcpy(host, hp->h_name);

i.e. it can write up to servlen / hostlen bytes PLUS a terminating zero.
This contradicts the manpage (and RFC 2533) as well as the way your own
programs appear to use it (at least OpenSSH and in.ftpd use sizeof() of a
buffer as servlen / hostlen).


Proposed fix:

Replace >'s with >='s.


Related problems:

The simpleminded implementation of getnameinfo() included in "portable
OpenSSH" is affected too.


--Pavel Kankovsky aka Peak  [ Boycott Microsoft--http://www.vcnet.com/bms ]
"Resistance is futile. Open your source code and prepare for
assimilation."

>getnameinfo() (confirmed for CVS version 1.14.2.1) does this sort of
>buffer size checks (these is just two of many cases):
	thanks, openbsd-current is corrected.  will ask for update to
	2.7-STABLE.

itojun

On Tue, 26 Sep 2000, Pavel Kankovsky wrote:
> Description:
> 
> getnameinfo() (confirmed for CVS version 1.14.2.1) does this sort of
> buffer size checks (these is just two of many cases):
 
[snip]
> i.e. it can write up to servlen / hostlen bytes PLUS a terminating zero.
> This contradicts the manpage (and RFC 2533) as well as the way your own
> programs appear to use it (at least OpenSSH and in.ftpd use sizeof() of a
> buffer as servlen / hostlen).
> 
> 
> Proposed fix:
> 
> Replace >'s with >='s.
 
Thanks for the report. This has now been fixed in the portable version.
I'll try to get a release out ASAP. Until then:

diff -u -r1.3 fake-getnameinfo.c
--- fake-getnameinfo.c	2000/05/31 01:20:12	1.3
+++ fake-getnameinfo.c	2000/09/26 02:12:50
@@ -30,7 +30,7 @@
 
 	if (host) {
 		if (flags & NI_NUMERICHOST) {
-			if (strlen(inet_ntoa(sin->sin_addr)) > hostlen)
+			if (strlen(inet_ntoa(sin->sin_addr)) >= hostlen)
 				return EAI_MEMORY;
 
 			strcpy(host, inet_ntoa(sin->sin_addr));
@@ -41,7 +41,7 @@
 			if (hp == NULL)
 				return EAI_NODATA;
 			
-			if (strlen(hp->h_name) > hostlen)
+			if (strlen(hp->h_name) >= hostlen)
 				return EAI_MEMORY;
 
 			strcpy(host, hp->h_name);



-- 
| ``The power of accurate observation is  | Damien Miller <djm at
mindrot.org>
| commonly called cynicism by those who   | @Work <djm at ibs.com.au>
| have not got it'' - George Bernard Shaw | http://www.mindrot.org