From dc8ed83892405ee6f107f989678f1018b2649512 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Sep 2024 17:43:28 +0200 Subject: [PATCH 1/3] dns-domain: follow our current variable naming style --- src/shared/dns-domain.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index ba24a77dad..62ec8de41d 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -395,9 +395,9 @@ int dns_label_undo_idna(const char *encoded, size_t encoded_size, char *decoded, } #endif -int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **_ret) { - _cleanup_free_ char *ret = NULL; - size_t n = 0; +int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **ret) { + _cleanup_free_ char *result = NULL; + size_t n_result = 0; const char *p; bool first = true; int r; @@ -428,16 +428,16 @@ int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **_r break; } - if (_ret) { - if (!GREEDY_REALLOC(ret, n + !first + DNS_LABEL_ESCAPED_MAX)) + if (ret) { + if (!GREEDY_REALLOC(result, n_result + !first + DNS_LABEL_ESCAPED_MAX)) return -ENOMEM; - r = dns_label_escape(label, r, ret + n + !first, DNS_LABEL_ESCAPED_MAX); + r = dns_label_escape(label, r, result + n_result + !first, DNS_LABEL_ESCAPED_MAX); if (r < 0) return r; if (!first) - ret[n] = '.'; + result[n_result] = '.'; } else { char escaped[DNS_LABEL_ESCAPED_MAX]; @@ -446,28 +446,28 @@ int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **_r return r; } - n += r + !first; + n_result += r + !first; first = false; } finish: - if (n > DNS_HOSTNAME_MAX) + if (n_result > DNS_HOSTNAME_MAX) return -EINVAL; - if (_ret) { - if (n == 0) { + if (ret) { + if (n_result == 0) { /* Nothing appended? If so, generate at least a single dot, to indicate the DNS root domain */ - if (!GREEDY_REALLOC(ret, 2)) + if (!GREEDY_REALLOC(result, 2)) return -ENOMEM; - ret[n++] = '.'; + result[n_result++] = '.'; } else { - if (!GREEDY_REALLOC(ret, n + 1)) + if (!GREEDY_REALLOC(result, n_result + 1)) return -ENOMEM; } - ret[n] = 0; - *_ret = TAKE_PTR(ret); + result[n_result] = 0; + *ret = TAKE_PTR(result); } return 0; From 1e1661c5d24365c97968a28ccb4f36dbf65f1542 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Sep 2024 17:54:06 +0200 Subject: [PATCH 2/3] dns-domain: validate dns domain name max size based on unescaped, not escaped size Otherwise we'll consider various domains invalid that really shouldn't be considered invalid. Fixes: #34399 --- src/shared/dns-domain.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 62ec8de41d..3635167b33 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -397,7 +397,7 @@ int dns_label_undo_idna(const char *encoded, size_t encoded_size, char *decoded, int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **ret) { _cleanup_free_ char *result = NULL; - size_t n_result = 0; + size_t n_result = 0, n_unescaped = 0; const char *p; bool first = true; int r; @@ -427,6 +427,7 @@ int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **re break; } + n_unescaped += r + !first; /* Count unescaped length to make max length determination below */ if (ret) { if (!GREEDY_REALLOC(result, n_result + !first + DNS_LABEL_ESCAPED_MAX)) @@ -451,21 +452,27 @@ int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **re } finish: - if (n_result > DNS_HOSTNAME_MAX) - return -EINVAL; + if (n_unescaped == 0) { + /* Nothing appended? If so, generate at least a single dot, to indicate the DNS root domain */ - if (ret) { - if (n_result == 0) { - /* Nothing appended? If so, generate at least a single dot, to indicate the DNS root domain */ - if (!GREEDY_REALLOC(result, 2)) + if (ret) { + if (!GREEDY_REALLOC(result, 2)) /* Room for dot, and already pre-allocate space for the trailing NUL byte at the same time */ return -ENOMEM; result[n_result++] = '.'; - } else { - if (!GREEDY_REALLOC(result, n_result + 1)) - return -ENOMEM; } + n_unescaped++; + } + + if (n_unescaped > DNS_HOSTNAME_MAX) /* Enforce max length check on unescaped length */ + return -EINVAL; + + if (ret) { + /* Suffix with a NUL byte */ + if (!GREEDY_REALLOC(result, n_result + 1)) + return -ENOMEM; + result[n_result] = 0; *ret = TAKE_PTR(result); } From 3f49d589201650d1c5125874223ddf298e075b07 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Sep 2024 17:45:58 +0200 Subject: [PATCH 3/3] dns-domain: add test case from #34399 --- src/test/test-dns-domain.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index d775601dae..8088055295 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -375,6 +375,9 @@ static void test_dns_name_is_valid_one(const char *s, int ret, int ret_ldh) { } TEST(dns_name_is_valid) { + test_dns_name_is_valid_one("[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[._qotd._tcp.local", 1, 0); + test_dns_name_is_valid_one("[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]._qotd._tcp.local", 0, 0); + test_dns_name_is_valid_one("foo", 1, 1); test_dns_name_is_valid_one("foo.", 1, 1); test_dns_name_is_valid_one("foo..", 0, 0);