diff options
author | Florian Obser <florian@cvs.openbsd.org> | 2022-11-17 17:39:42 +0000 |
---|---|---|
committer | Florian Obser <florian@cvs.openbsd.org> | 2022-11-17 17:39:42 +0000 |
commit | b849041b941c5336f31303ea64091bbb0377cd7b (patch) | |
tree | d25a7f66d237f023d8d4b03e2c2c0b2863bc4a1f | |
parent | 2009307ba16f783871c992f2a43a9a914e3cde16 (diff) |
Restrict what getaddrinfo(3) is willing to try to resolve.
Programs assume that a successful call to getaddrinfo(3) validates the
input as "safe", but that's not true. Characters like '$', '`', '\n'
or '*' can traverse the DNS without problems, but have special
meaning, for example a shell.
There is a function res_hnok() already in libc, but it validates if a
string is a host name, which is too strict in practice. For example
foo-.example.com is not a valid host name, but is used on the
Internet.
Posix has this to say:
"The getaddrinfo() function shall translate the name of a service
location (for example, a host name)"
It hints that the input should be a host name, but it does not
restrict it to it.
This introduces a function hnok_lenient() which restricts the input to
getaddrinfo(3) to the set [A-z0-9-_.]. Additionally two consecutive
dots ('.') are not allowed nor can the string start with - or '.'.
glibc introduced a similar restriction years ago, so this should not
cause problems.
It has been known in the DNS community for years, probably decades
that getaddrinfo(3) is too lenient what it accepts, but it has always
been kicked down the road as "not a DNS problem". Unfortunately this
information never made it out of the DNS community and no coordinated
effort happened to have this addressed in operating systems.
David Leadbeater recently demonstrated how ssh(1) and ftp(1) are too
trusting with what getaddrinfo(3) accepts. Both have been fixed
independently of this.
Input deraadt, eric
OK millert, deraadt
-rw-r--r-- | lib/libc/asr/asr_private.h | 3 | ||||
-rw-r--r-- | lib/libc/asr/asr_utils.c | 21 | ||||
-rw-r--r-- | lib/libc/asr/getaddrinfo_async.c | 11 | ||||
-rw-r--r-- | lib/libc/asr/gethostnamadr_async.c | 7 |
4 files changed, 37 insertions, 5 deletions
diff --git a/lib/libc/asr/asr_private.h b/lib/libc/asr/asr_private.h index 49123a3c246..64f044f12d1 100644 --- a/lib/libc/asr/asr_private.h +++ b/lib/libc/asr/asr_private.h @@ -1,4 +1,4 @@ -/* $OpenBSD: asr_private.h,v 1.47 2018/04/28 15:16:49 schwarze Exp $ */ +/* $OpenBSD: asr_private.h,v 1.48 2022/11/17 17:39:41 florian Exp $ */ /* * Copyright (c) 2012 Eric Faurot <eric@openbsd.org> * @@ -298,6 +298,7 @@ int _asr_unpack_rr(struct asr_unpack *, struct asr_dns_rr *); int _asr_sockaddr_from_str(struct sockaddr *, int, const char *); ssize_t _asr_dname_from_fqdn(const char *, char *, size_t); ssize_t _asr_addr_as_fqdn(const char *, int, char *, size_t); +int hnok_lenient(const char *); /* asr.c */ void _asr_resolver_done(void *); diff --git a/lib/libc/asr/asr_utils.c b/lib/libc/asr/asr_utils.c index c2cb64b4ddc..6009b07a3a0 100644 --- a/lib/libc/asr/asr_utils.c +++ b/lib/libc/asr/asr_utils.c @@ -1,4 +1,4 @@ -/* $OpenBSD: asr_utils.c,v 1.18 2017/09/23 20:55:06 jca Exp $ */ +/* $OpenBSD: asr_utils.c,v 1.19 2022/11/17 17:39:41 florian Exp $ */ /* * Copyright (c) 2009-2012 Eric Faurot <eric@faurot.net> * @@ -563,3 +563,22 @@ _asr_addr_as_fqdn(const char *addr, int family, char *dst, size_t max) } return (0); } + +int +hnok_lenient(const char *dn) +{ + int pch = '\0', ch = *dn++; + + while (ch != '\0') { + /* can't start with . or - */ + if (pch == '\0' && (ch == '.' || ch == '-')) + return 0; + if (pch == '.' && ch == '.') + return 0; + if (!(isalpha((unsigned char)ch) || isdigit((unsigned char)ch) || + ch == '.' || ch == '-' || ch == '_')) + return 0; + pch = ch; ch = *dn++; + } + return 1; +} diff --git a/lib/libc/asr/getaddrinfo_async.c b/lib/libc/asr/getaddrinfo_async.c index 5fdfa4985e6..b351a867f11 100644 --- a/lib/libc/asr/getaddrinfo_async.c +++ b/lib/libc/asr/getaddrinfo_async.c @@ -1,4 +1,4 @@ -/* $OpenBSD: getaddrinfo_async.c,v 1.57 2021/01/26 12:27:28 florian Exp $ */ +/* $OpenBSD: getaddrinfo_async.c,v 1.58 2022/11/17 17:39:41 florian Exp $ */ /* * Copyright (c) 2012 Eric Faurot <eric@openbsd.org> * @@ -146,7 +146,7 @@ getaddrinfo_async_run(struct asr_query *as, struct asr_result *ar) async_set_state(as, ASR_STATE_HALT); break; } - + ai = &as->as.ai.hints; if (ai->ai_addrlen || @@ -281,6 +281,13 @@ getaddrinfo_async_run(struct asr_query *as, struct asr_result *ar) break; } + /* make sure there are no funny characters in hostname */ + if (!hnok_lenient(as->as.ai.hostname)) { + ar->ar_gai_errno = EAI_FAIL; + async_set_state(as, ASR_STATE_HALT); + break; + } + async_set_state(as, ASR_STATE_NEXT_DB); break; diff --git a/lib/libc/asr/gethostnamadr_async.c b/lib/libc/asr/gethostnamadr_async.c index ecda0ffb5d4..e40811b69ba 100644 --- a/lib/libc/asr/gethostnamadr_async.c +++ b/lib/libc/asr/gethostnamadr_async.c @@ -1,4 +1,4 @@ -/* $OpenBSD: gethostnamadr_async.c,v 1.45 2019/06/27 05:26:37 martijn Exp $ */ +/* $OpenBSD: gethostnamadr_async.c,v 1.46 2022/11/17 17:39:41 florian Exp $ */ /* * Copyright (c) 2012 Eric Faurot <eric@openbsd.org> * @@ -202,6 +202,11 @@ gethostnamadr_async_run(struct asr_query *as, struct asr_result *ar) } async_set_state(as, ASR_STATE_HALT); break; + } else { + if (!hnok_lenient(as->as.hostnamadr.name)) { + ar->ar_gai_errno = EAI_FAIL; + async_set_state(as, ASR_STATE_HALT); + } } } async_set_state(as, ASR_STATE_NEXT_DB); |