From 1b04c78cb3ee7608c76cbb26114b603e2d854d11 Mon Sep 17 00:00:00 2001 From: florian Date: Thu, 17 Nov 2022 17:39:41 +0000 Subject: [PATCH] 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 --- lib/libc/asr/asr_private.h | 3 ++- lib/libc/asr/asr_utils.c | 21 ++++++++++++++++++++- lib/libc/asr/getaddrinfo_async.c | 11 +++++++++-- 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 * @@ -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 * @@ -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 * @@ -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 * @@ -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); -- 2.20.1