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

diff --git a/lib/libc/asr/asr_private.h b/lib/libc/asr/asr_private.h
index 49123a3..64f044f 100644 (file)
--- 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 c2cb64b..6009b07 100644 (file)
--- 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 5fdfa49..b351a86 100644 (file)
--- 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 ecda0ff..e40811b 100644 (file)
--- 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);