From 9db01ca5b0322bc035e1ccd6b8a0d98a26533b4a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 11 Jun 2022 05:51:03 +0900 Subject: [PATCH 1/3] dns-domain: make each label nul-terminated dns_label_unescape() does not nul-terminate the buffer if it does not have enough space. Hence, if a lable is enough long, then strjoin() triggers buffer-overflow. Fixes #23705. --- src/shared/dns-domain.c | 2 +- src/test/test-dns-domain.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 9acf1cb0d6..97839e7d73 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1025,7 +1025,7 @@ static bool dns_service_name_label_is_valid(const char *label, size_t n) { int dns_service_split(const char *joined, char **_name, char **_type, char **_domain) { _cleanup_free_ char *name = NULL, *type = NULL, *domain = NULL; const char *p = joined, *q = NULL, *d = NULL; - char a[DNS_LABEL_MAX], b[DNS_LABEL_MAX], c[DNS_LABEL_MAX]; + char a[DNS_LABEL_MAX+1], b[DNS_LABEL_MAX+1], c[DNS_LABEL_MAX+1]; int an, bn, cn, r; unsigned x = 0; diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index 3a26ecaa70..0fbac9dbd9 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -540,6 +540,7 @@ TEST(dns_service_split) { test_dns_service_split_one("_foo._bar", NULL, "_foo._bar", ".", 0); test_dns_service_split_one("_meh._foo._bar", "_meh", "_foo._bar", ".", 0); test_dns_service_split_one("Wuff\\032Wuff._foo._bar.waldo.com", "Wuff Wuff", "_foo._bar", "waldo.com", 0); + test_dns_service_split_one("_Q._Q-------------------------------------------------------------", NULL, "_Q._Q-------------------------------------------------------------", ".", 0); } static void test_dns_name_change_suffix_one(const char *name, const char *old_suffix, const char *new_suffix, int r, const char *result) { From ec4c01475e88c07bd447cee8faab9e0e7900ead5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 11 Jun 2022 05:28:26 +0900 Subject: [PATCH 2/3] dns-domain: rename function arguments --- src/shared/dns-domain.c | 14 +++++++------- src/shared/dns-domain.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 97839e7d73..20abb84f0f 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1022,7 +1022,7 @@ static bool dns_service_name_label_is_valid(const char *label, size_t n) { return dns_service_name_is_valid(s); } -int dns_service_split(const char *joined, char **_name, char **_type, char **_domain) { +int dns_service_split(const char *joined, char **ret_name, char **ret_type, char **ret_domain) { _cleanup_free_ char *name = NULL, *type = NULL, *domain = NULL; const char *p = joined, *q = NULL, *d = NULL; char a[DNS_LABEL_MAX+1], b[DNS_LABEL_MAX+1], c[DNS_LABEL_MAX+1]; @@ -1103,14 +1103,14 @@ finish: if (r < 0) return r; - if (_domain) - *_domain = TAKE_PTR(domain); + if (ret_domain) + *ret_domain = TAKE_PTR(domain); - if (_type) - *_type = TAKE_PTR(type); + if (ret_type) + *ret_type = TAKE_PTR(type); - if (_name) - *_name = TAKE_PTR(name); + if (ret_name) + *ret_name = TAKE_PTR(name); return 0; } diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index 08cb1cc2aa..5421c60ee7 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -88,7 +88,7 @@ bool dnssd_srv_type_is_valid(const char *name); bool dns_service_name_is_valid(const char *name); int dns_service_join(const char *name, const char *type, const char *domain, char **ret); -int dns_service_split(const char *joined, char **name, char **type, char **domain); +int dns_service_split(const char *joined, char **ret_name, char **ret_type, char **ret_domain); int dns_name_suffix(const char *name, unsigned n_labels, const char **ret); int dns_name_count_labels(const char *name); From 3c4ae50abe6f53071e07adeda75b84f8ae63e843 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 11 Jun 2022 05:48:35 +0900 Subject: [PATCH 3/3] dns-domain: reduce indentation in dns_service_split() --- src/shared/dns-domain.c | 79 ++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 20abb84f0f..d202fe5d86 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1024,7 +1024,7 @@ static bool dns_service_name_label_is_valid(const char *label, size_t n) { int dns_service_split(const char *joined, char **ret_name, char **ret_type, char **ret_domain) { _cleanup_free_ char *name = NULL, *type = NULL, *domain = NULL; - const char *p = joined, *q = NULL, *d = NULL; + const char *p = joined, *q = NULL, *d = joined; char a[DNS_LABEL_MAX+1], b[DNS_LABEL_MAX+1], c[DNS_LABEL_MAX+1]; int an, bn, cn, r; unsigned x = 0; @@ -1045,6 +1045,9 @@ int dns_service_split(const char *joined, char **ret_name, char **ret_type, char return bn; if (bn > 0) { + if (!srv_type_label_is_valid(b, bn)) + goto finish; + x++; /* If there was a second label, try to get the third one */ @@ -1053,50 +1056,44 @@ int dns_service_split(const char *joined, char **ret_name, char **ret_type, char if (cn < 0) return cn; - if (cn > 0) + if (cn > 0 && srv_type_label_is_valid(c, cn)) x++; - } else - cn = 0; - } else - an = 0; - - if (x >= 2 && srv_type_label_is_valid(b, bn)) { - - if (x >= 3 && srv_type_label_is_valid(c, cn)) { - - if (dns_service_name_label_is_valid(a, an)) { - /* OK, got . . . */ - - name = strndup(a, an); - if (!name) - return -ENOMEM; - - type = strjoin(b, ".", c); - if (!type) - return -ENOMEM; - - d = p; - goto finish; - } - - } else if (srv_type_label_is_valid(a, an)) { - - /* OK, got . . */ - - name = NULL; - - type = strjoin(a, ".", b); - if (!type) - return -ENOMEM; - - d = q; - goto finish; } } - name = NULL; - type = NULL; - d = joined; + switch (x) { + case 2: + if (!srv_type_label_is_valid(a, an)) + break; + + /* OK, got . . */ + + name = NULL; + + type = strjoin(a, ".", b); + if (!type) + return -ENOMEM; + + d = q; + break; + + case 3: + if (!dns_service_name_label_is_valid(a, an)) + break; + + /* OK, got . . . */ + + name = strndup(a, an); + if (!name) + return -ENOMEM; + + type = strjoin(b, ".", c); + if (!type) + return -ENOMEM; + + d = p; + break; + } finish: r = dns_name_normalize(d, 0, &domain);