NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: lib/48881: /etc/hosts file parsing broken (since Aug 2013)
The following reply was made to PR lib/48881; it has been noted by GNATS.
From: "Greg A. Woods" <woods%planix.ca@localhost>
To: NetBSD GNATS <gnats-bugs%NetBSD.org@localhost>
Cc:
Subject: Re: lib/48881: /etc/hosts file parsing broken (since Aug 2013)
Date: Fri, 13 Jun 2014 15:03:49 -0700
Appended is a new improved patch that would fully fix the bug for
netbsd-5 (but the patch is against -current, and fixes the missing final
newline ase for -current), and it includes some new test cases for
-current.
I was of course unable to reproduce the original bug in in -current.
The bug is only really nasty in the netbsd-5 branch (since NetBSD-5.2).
It disappears (in an un-documented way) in -current because of changes
in the implementation fgetln().
In netbsd-5 writing the NUL byte past the length returned by fgetln()
tromps on the first character of the next line and so that line is
treated as empty (the next strpbrk() can't find any newline or '#').
Only a blank or comment line in the input will "accidentally" reset the
bug and allow the code to see the full next line and confuse the heck
out of the person trying to figure out the problem.
The code could probably use getdelim() or getline() instead of fgetln()
if it really wants to assume the result is a C string. Otherwise it
should follow the rules for portable use of fgetln(). Better to be
clearly safe than sorry.
However the code still doesn't work correctly for the missing final
newline case (i.e. the last entry in a hosts file with no final newline
on that last entry cannot be found by name or by address).
The extra mallocs could be avoided in all but the missing final newline
case I suppose (while still using fgetln()), but at the expense of more
complex code. However now that the aliases array is also malloced there
doesn't seem to be any point to trying to avoid using malloc in this
code, and it's not likely anyone will have huge numbers of entries in
/etc/hosts (and thus huge numbers of mallocs) anyway.
--=20
Greg A. Woods
Planix, Inc.
<woods%planix.com@localhost> +1 250 762-7675
http://www.planix.com/
Index: tests/lib/libc/net/t_hostent.sh
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/master/m-NetBSD/main/src/tests/lib/libc/net/t_hostent.sh,v
retrieving revision 1.10
diff -u -r1.10 t_hostent.sh
--- tests/lib/libc/net/t_hostent.sh 13 Jan 2014 11:08:14 -0000 1.10
+++ tests/lib/libc/net/t_hostent.sh 13 Jun 2014 20:49:05 -0000
@@ -41,6 +41,30 @@
al4=3D"127.0.0.1"
loc4=3D"name=3D$l4, length=3D4, addrtype=3D2, aliases=3D[localhost. localh=
ost.localdomain.] addr_list=3D[$al4]\n"
=20
+# Many more combinations should be provided in hosts and tried here,
+# but this should scratch most major features and edge cases, though
+# just one at a time....
+
+f4=3D"oldfoohost.local"
+af4=3D"10.0.1.1"
+resf4=3D"name=3Dfoohost.local, length=3D4, addrtype=3D2, aliases=3D[$f4] a=
ddr_list=3D[$af4]\n"
+
+cm4=3D"commented.local"
+acm4=3D"10.0.2.2"
+rescm4=3D"name=3Dcommented, length=3D4, addrtype=3D2, aliases=3D[$cm4] add=
r_list=3D[$acm4]\n"
+
+sp4=3D"spaced.local"
+asp4=3D"10.0.3.3"
+ressp4=3D"name=3D$sp4, length=3D4, addrtype=3D2, aliases=3D[] addr_list=3D=
[$asp4]\n"
+
+msp4=3D"multispaced.local"
+amsp4=3D"10.0.4.4"
+resmsp4=3D"name=3D$msp4, length=3D4, addrtype=3D2, aliases=3D[] addr_list=
=3D[$amsp4]\n"
+
+nnl4=3D"nonewline.local"
+annl4=3D"10.10.10.10"
+resnnl4=3D"name=3Dnonewline, length=3D4, addrtype=3D2, aliases=3D[$nnl4] a=
ddr_list=3D[$annl4]\n"
+
dir=3D"$(atf_get_srcdir)"
res=3D"-r ${dir}/resolv.conf"
=20
@@ -131,6 +155,56 @@
atf_check -o inline:"$loc4" -x "${dir}/h_hostent -f ${dir}/hosts -t
file =
-4 $l4"
}
=20
+atf_test_case hostsbynamelookup4a
+hostsbynamelookup4a_head()
+{
+ atf_set "descr" "Checks /etc/hosts name lookup by alias for AF_INET"
+}
+hostsbynamelookup4a_body()
+{
+ atf_check -o inline:"$resf4" -x "${dir}/h_hostent -f ${dir}/hosts -t
file=
-4 $f4"
+}
+
+atf_test_case hostsbynamelookup4b
+hostsbynamelookup4b_head()
+{
+ atf_set "descr" "Checks /etc/hosts name lookup with comment for AF_INET"
+}
+hostsbynamelookup4b_body()
+{
+ atf_check -o inline:"$rescm4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fil=
e -4 $cm4"
+}
+
+atf_test_case hostsbynamelookup4c
+hostsbynamelookup4c_head()
+{
+ atf_set "descr" "Checks /etc/hosts name lookup with space for AF_INET"
+}
+hostsbynamelookup4c_body()
+{
+ atf_check -o inline:"$ressp4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fil=
e -4 $sp4"
+}
+
+atf_test_case hostsbynamelookup4d
+hostsbynamelookup4d_head()
+{
+ atf_set "descr" "Checks /etc/hosts name lookup with multiple spaces for
A=
F_INET"
+}
+hostsbynamelookup4d_body()
+{
+ atf_check -o inline:"$resmsp4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fi=
le -4 $msp4"
+}
+
+atf_test_case hostsbynamelookup4e
+hostsbynamelookup4e_head()
+{
+ atf_set "descr" "Checks /etc/hosts name lookup with no newline for
AF_INE=
T"
+}
+hostsbynamelookup4e_body()
+{
+ atf_check -o inline:"$resnnl4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fi=
le -4 $nnl4"
+}
+
atf_test_case hostsbynamelookup6
hostsbynamelookup6_head()
{
@@ -151,6 +225,56 @@
atf_check -o inline:"$loc4" -x "${dir}/h_hostent -f ${dir}/hosts -t
file =
-4 -a $al4"
}
=20
+atf_test_case hostsbyaddrlookup4a
+hostsbyaddrlookup4a_head()
+{
+ atf_set "descr" "Checks /etc/hosts address lookup again for AF_INET"
+}
+hostsbyaddrlookup4a_body()
+{
+ atf_check -o inline:"$resf4" -x "${dir}/h_hostent -f ${dir}/hosts -t
file=
-4 -a $af4"
+}
+
+atf_test_case hostsbyaddrlookup4b
+hostsbyaddrlookup4b_head()
+{
+ atf_set "descr" "Checks /etc/hosts address lookup with comment for
AF_INE=
T"
+}
+hostsbyaddrlookup4b_body()
+{
+ atf_check -o inline:"$rescm4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fil=
e -4 -a $acm4"
+}
+
+atf_test_case hostsbyaddrlookup4c
+hostsbyaddrlookup4c_head()
+{
+ atf_set "descr" "Checks /etc/hosts address lookup with space for
AF_INET"
+}
+hostsbyaddrlookup4c_body()
+{
+ atf_check -o inline:"$ressp4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fil=
e -4 -a $asp4"
+}
+
+atf_test_case hostsbyaddrlookup4d
+hostsbyaddrlookup4d_head()
+{
+ atf_set "descr" "Checks /etc/hosts address lookup with multiple spaces
fo=
r AF_INET"
+}
+hostsbyaddrlookup4d_body()
+{
+ atf_check -o inline:"$resmsp4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fi=
le -4 -a $amsp4"
+}
+
+atf_test_case hostsbyaddrlookup4e
+hostsbyaddrlookup4e_head()
+{
+ atf_set "descr" "Checks /etc/hosts address lookup with no newline for
AF_=
INET"
+}
+hostsbyaddrlookup4e_body()
+{
+ atf_check -o inline:"$resnnl4" -x "${dir}/h_hostent -f ${dir}/hosts -t
fi=
le -4 -a $annl4"
+}
+
atf_test_case hostsbyaddrlookup6
hostsbyaddrlookup6_head()
{
@@ -229,8 +353,18 @@
atf_add_test_case gethostbyaddr6
=20
atf_add_test_case hostsbynamelookup4
+ atf_add_test_case hostsbynamelookup4a
+ atf_add_test_case hostsbynamelookup4b
+ atf_add_test_case hostsbynamelookup4c
+ atf_add_test_case hostsbynamelookup4d
+ atf_add_test_case hostsbynamelookup4e
atf_add_test_case hostsbynamelookup6
atf_add_test_case hostsbyaddrlookup4
+ atf_add_test_case hostsbyaddrlookup4a
+ atf_add_test_case hostsbyaddrlookup4b
+ atf_add_test_case hostsbyaddrlookup4c
+ atf_add_test_case hostsbyaddrlookup4d
+ atf_add_test_case hostsbyaddrlookup4e
atf_add_test_case hostsbyaddrlookup6
=20
atf_add_test_case dnsbynamelookup4
Index: tests/lib/libc/net/hosts
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/master/m-NetBSD/main/src/tests/lib/libc/net/hosts,v
retrieving revision 1.1
diff -u -r1.1 hosts
--- tests/lib/libc/net/hosts 16 Aug 2013 15:29:45 -0000 1.1
+++ tests/lib/libc/net/hosts 13 Jun 2014 20:55:14 -0000
@@ -1,3 +1,5 @@
+# -*- eval (setq require-final-newline nil) -*-
+#
# $NetBSD: hosts,v 1.1 2013/08/16 15:29:45 christos Exp $
#
# Host Database
@@ -9,3 +11,19 @@
#
::1 localhost localhost. localhost.localdomain.
127.0.0.1 localhost localhost. localhost.localdomain.
+
+# some test hosts...
+
+10.0.0.0 network network.localdomain
+
+10.0.1.1 foohost.local oldfoohost.local
+10.0.2.2 commented commented.local # trailing comment
+10.0.3.3 spaced.local
+10.0.4.4 multispaced.local
+
+10.255.255.255 broadcast broadcast.localdomain
+
+# N.B.: no trailing newline!!!
+# (non-emacs users do this: printf %s "$(< hosts)" > t && mv t hosts)
+# xxx or maybe the test setup should do it....
+10.10.10.10 nonewline nonewline.local
\ No newline at end of file
Index: lib/libc/net/gethnamaddr.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/master/m-NetBSD/main/src/lib/libc/net/gethnamaddr.c,v
retrieving revision 1.90
diff -u -r1.90 gethnamaddr.c
--- lib/libc/net/gethnamaddr.c 24 Jan 2014 17:26:18 -0000 1.90
+++ lib/libc/net/gethnamaddr.c 13 Jun 2014 20:28:45 -0000
@@ -83,8 +83,6 @@
# define LOG_AUTH 0
#endif
=20
-#define MULTI_PTRS_ARE_ALIASES 1 /* XXX - experimental */
-
#include <nsswitch.h>
#include <stdlib.h>
#include <string.h>
@@ -378,7 +376,6 @@
had_error++;
break;
}
-#if MULTI_PTRS_ARE_ALIASES
cp +=3D n;
if (cp !=3D erdata)
goto no_recovery;
@@ -395,19 +392,6 @@
bp +=3D n;
}
break;
-#else
- hent->h_name =3D bp;
- if (res->options & RES_USE_INET6) {
- n =3D strlen(bp) + 1; /* for the \0 */
- if (n >=3D MAXHOSTNAMELEN) {
- had_error++;
- break;
- }
- bp +=3D n;
- map_v4v6_hostent(hent, &bp, ep);
- }
- goto success;
-#endif
case T_A:
case T_AAAA:
if (strcasecmp(hent->h_name, bp) !=3D 0) {
@@ -738,7 +722,7 @@
struct hostent *
gethostent_r(FILE *hf, struct hostent *hent, char *buf, size_t buflen, int=
*he)
{
- char *p, *name;
+ char *ln, *p, *name;
char *cp, **q;
int af, len;
size_t llen, anum;
@@ -753,21 +737,30 @@
}
setup(aliases, maxaliases);
again:
- if ((p =3D fgetln(hf, &llen)) =3D=3D NULL) {
+ if ((ln =3D fgetln(hf, &llen)) =3D=3D NULL) {
free(aliases);
*he =3D HOST_NOT_FOUND;
return NULL;
}
if (llen < 1)
goto again;
- if (*p =3D=3D '#')
+ if (*ln =3D=3D '#')
goto again;
+ /* xxx we only really need to alloc and copy IFF no newline */
+ if (! (p =3D malloc(llen + 1))) {
+ goto nospc;
+ }
+ memcpy(p, ln, llen);
p[llen] =3D '\0';
- if (!(cp =3D strpbrk(p, "#\n")))
- goto again;
- *cp =3D '\0';
- if (!(cp =3D strpbrk(p, " \t")))
+ if ((cp =3D strpbrk(p, "#\n")) !=3D NULL) {
+ /* trim any comment to EOL, or the newline */
+ *cp =3D '\0';
+ }
+ if (!(cp =3D strpbrk(p, " \t"))) {
+ /* there must be at least two words on a line */
+ free(p);
goto again;
+ }
*cp++ =3D '\0';
if (inet_pton(AF_INET6, p, &host_addr) > 0) {
af =3D AF_INET6;
@@ -786,14 +779,18 @@
}
__res_put_state(res);
} else {
+ free(p);
goto again;
}
/* if this is not something we're looking for, skip it. */
- if (hent->h_addrtype !=3D 0 && hent->h_addrtype !=3D af)
+ if (hent->h_addrtype !=3D 0 && hent->h_addrtype !=3D af) {
+ free(p);
goto again;
- if (hent->h_length !=3D 0 && hent->h_length !=3D len)
+ }
+ if (hent->h_length !=3D 0 && hent->h_length !=3D len) {
+ free(p);
goto again;
-
+ }
while (*cp =3D=3D ' ' || *cp =3D=3D '\t')
cp++;
if ((cp =3D strpbrk(name =3D cp, " \t")) !=3D NULL)
@@ -823,6 +820,7 @@
hent->h_aliases[anum] =3D NULL;
=20
*he =3D NETDB_SUCCESS;
+ free(p);
free(aliases);
return hent;
nospc:
Home |
Main Index |
Thread Index |
Old Index