March 2018 - tor-commits - lists.torproject.org

[tor/master] Improve handling of trailing dot
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 4413e52f9e2e3898f870df481aea93b1ea5fd836 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 12 22:21:10 2018 +0100 Improve handling of trailing dot --- src/common/util.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index a7eaf5389..096188cfc 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1106,24 +1106,33 @@ int string_is_valid_hostname(const char *string) { int result = 1; + int has_trailing_dot; + char *last_label; smartlist_t *components; + if (!string || strlen(string) == 0) + return 0; + components = smartlist_new(); smartlist_split_string(components,string,".",0,0); + /* Allow a single terminating '.' used rarely to indicate domains + * are FQDNs rather than relative. */ + last_label = (char *)smartlist_get(components, smartlist_len(components) - 1); + has_trailing_dot = (last_label[0] == '\0'); + if (has_trailing_dot) { + smartlist_pop_last(components); + tor_free(last_label); + last_label = NULL; + } + SMARTLIST_FOREACH_BEGIN(components, char *, c) { if ((c[0] == '-') || (*c == '_')) { result = 0; break; } - /* Allow a single terminating '.' used rarely to indicate domains - * are FQDNs rather than relative. */ - if ((c_sl_idx > 0) && (c_sl_idx + 1 == c_sl_len) && !*c) { - continue; - } - if (c_sl_idx == c_sl_len - 1) { do { result = TOR_ISALPHA(*c);

1 0

[tor/master] Also test bracket-less IPv6 string validation
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 12afd8bfed96c57cd47b18644c2030673496a74f Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 12 21:16:38 2018 +0100 Also test bracket-less IPv6 string validation --- src/test/test_socks.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/test_socks.c b/src/test/test_socks.c index 3f9cc887b..8da7191e8 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -365,6 +365,23 @@ test_socks_5_supported_commands(void *ptr) socks_request_clear(socks); + /* Also allow bracket-less form. */ + + ADD_DATA(buf, "\x05\x01\x00"); + ADD_DATA(buf, "\x05\xF0\x00\x03\x27"); + ADD_DATA(buf, "2001:0db8:85a3:0000:0000:8a2e:0370:7334"); + ADD_DATA(buf, "\x01\x02"); + tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, 1), + OP_EQ, 1); + + tt_str_op("2001:0db8:85a3:0000:0000:8a2e:0370:7334", OP_EQ, + socks->address); + tt_int_op(258, OP_EQ, socks->port); + + tt_int_op(0, OP_EQ, buf_datalen(buf)); + + socks_request_clear(socks); + /* SOCKS 5 Send RESOLVE_PTR [F1] for IP address 2.2.2.5 */ ADD_DATA(buf, "\x05\x01\x00"); ADD_DATA(buf, "\x05\xF1\x00\x01\x02\x02\x02\x05\x01\x03");

1 0

[tor/master] Refrain from including <ctype.h>
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 6335db9fce4275838c7de4bc10e522eb21a21ed8 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 12 21:26:57 2018 +0100 Refrain from including <ctype.h> --- src/common/util.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index d8891c6a5..a7eaf5389 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -100,8 +100,6 @@ #undef MALLOC_ZERO_WORKS #endif -#include <ctype.h> - /* ===== * Memory management * ===== */ @@ -1128,12 +1126,12 @@ string_is_valid_hostname(const char *string) if (c_sl_idx == c_sl_len - 1) { do { - result = isalpha(*c); + result = TOR_ISALPHA(*c); c++; } while (result && *c); } else { do { - result = (isalnum(*c) || (*c == '-') || (*c == '_')); + result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_')); c++; } while (result > 0 && *c); }

1 0

[tor/master] Don't explode on NULL or empty string
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 6b6d003f43cbbf01b40cedb0cc12ada2e81461f9 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Wed Feb 21 20:23:21 2018 +0100 Don't explode on NULL or empty string --- src/common/util.c | 9 +++++++-- src/test/test_util.c | 13 +++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 1402462fb..53e117f24 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1081,8 +1081,13 @@ string_is_valid_dest(const char *string) int retval; size_t len = strlen(string); - tor_assert(string); - tor_assert(len > 0); + if (string == NULL) + return 0; + + len = strlen(string); + + if (len == 0) + return 0; if (string[0] == '[' && string[len - 1] == ']') string = tmp = tor_strndup(string + 1, len - 2); diff --git a/src/test/test_util.c b/src/test/test_util.c index ee9b16494..c734426a5 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5542,6 +5542,18 @@ test_util_max_mem(void *arg) } static void +test_util_dest_validation_edgecase(void *arg) +{ + (void)arg; + + tt_assert(!string_is_valid_dest(NULL)); + tt_assert(!string_is_valid_dest("")); + + done: + return; +} + +static void test_util_hostname_validation(void *arg) { (void)arg; @@ -6222,6 +6234,7 @@ struct testcase_t util_tests[] = { &passthrough_setup, (void*)"1" }, UTIL_TEST(max_mem, 0), UTIL_TEST(hostname_validation, 0), + UTIL_TEST(dest_validation_edgecase, 0), UTIL_TEST(ipv4_validation, 0), UTIL_TEST(writepid, 0), UTIL_TEST(get_avail_disk_space, 0),

1 0

[tor/master] Tweak loop condition
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit a28e350cff1572a1e5a0c5df93f6e6005904689a Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Wed Feb 21 20:25:24 2018 +0100 Tweak loop condition --- src/common/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/util.c b/src/common/util.c index 53e117f24..497b60be1 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1145,7 +1145,7 @@ string_is_valid_hostname(const char *string) do { result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_')); c++; - } while (result > 0 && *c); + } while (result && *c); if (result == 0) { break;

1 0

[tor/master] Test TLD validation
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit db850fec3ac402084a9036c0ea7b4523f1120eb1 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 12 22:20:45 2018 +0100 Test TLD validation --- src/test/test_util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/test_util.c b/src/test/test_util.c index 2fa03e5bc..db2ea1a34 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5589,6 +5589,12 @@ test_util_hostname_validation(void *arg) tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]")); tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e")); + // Last label of a hostname is required to be alphabetic according to + // RFC 1123 Section 2.1. + tt_assert(!string_is_valid_hostname("lucky.13")); + tt_assert(!string_is_valid_hostname("luck.y13")); + tt_assert(!string_is_valid_hostname("luck.y13.")); + done: return; }

1 0

[tor/master] Allow alphanumeric TLDs in test for now
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit d891010fdd4562e29a5a468232cd7b30430d7570 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Tue Feb 20 19:52:48 2018 +0100 Allow alphanumeric TLDs in test for now --- src/test/test_util.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index ef1f420fe..ee9b16494 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5589,11 +5589,10 @@ test_util_hostname_validation(void *arg) tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]")); tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e")); - // Last label of a hostname is required to be alphabetic according to - // RFC 1123 Section 2.1. - tt_assert(!string_is_valid_hostname("lucky.13")); - tt_assert(!string_is_valid_hostname("luck.y13")); - tt_assert(!string_is_valid_hostname("luck.y13.")); + // We allow alphanumeric TLDs. For discussion, see ticket #25055. + tt_assert(string_is_valid_hostname("lucky.13")); + tt_assert(string_is_valid_hostname("luck.y13")); + tt_assert(string_is_valid_hostname("luck.y13.")); // We allow punycode TLDs. For examples, see // http://data.iana.org/TLD/tlds-alpha-by-domain.txt

1 0

[tor/master] Simplify hostname validation code
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit ee1fca727cd739ba94c215a4a45a416bfcc8956e Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 19 21:08:51 2018 +0100 Simplify hostname validation code --- src/common/util.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index a55f7a3cd..1402462fb 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1113,6 +1113,9 @@ string_is_valid_hostname(const char *string) if (!string || strlen(string) == 0) return 0; + if (string_is_valid_ipv4_address(string)) + return 0; + components = smartlist_new(); smartlist_split_string(components,string,".",0,0); @@ -1134,25 +1137,10 @@ string_is_valid_hostname(const char *string) break; } - if (c_sl_idx == c_sl_len - 1) { // TLD validation. - int is_punycode = (strlen(c) > 4 && - (c[0] == 'X' || c[0] == 'x') && - (c[1] == 'N' || c[1] == 'n') && - c[2] == '-' && c[3] == '-'); - - if (is_punycode) - c += 4; - - do { - result = is_punycode ? TOR_ISALNUM(*c) : TOR_ISALPHA(*c); - c++; - } while (result && *c); - } else { // Regular hostname label validation. - do { - result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_')); - c++; - } while (result > 0 && *c); - } + do { + result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_')); + c++; + } while (result > 0 && *c); if (result == 0) { break;

1 0

[tor/master] Don't strlen before checking for NULL
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 09351c34e9bea4d29fb6f4ac8d40e3bee49e12fc Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Thu Feb 22 19:52:40 2018 +0100 Don't strlen before checking for NULL --- src/common/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/util.c b/src/common/util.c index 497b60be1..5ae4d0408 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1079,7 +1079,7 @@ string_is_valid_dest(const char *string) { char *tmp = NULL; int retval; - size_t len = strlen(string); + size_t len; if (string == NULL) return 0;

1 0

[tor/master] Validate hostnames with punycode TLDs correctly
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit dbb7c8e6fd757db51226a47a2e14f4fd1aaf60c3 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sat Feb 17 21:49:02 2018 +0100 Validate hostnames with punycode TLDs correctly --- src/common/util.c | 17 +++++++++++++---- src/test/test_util.c | 4 ++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 096188cfc..a55f7a3cd 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1119,7 +1119,8 @@ string_is_valid_hostname(const char *string) /* Allow a single terminating '.' used rarely to indicate domains * are FQDNs rather than relative. */ - last_label = (char *)smartlist_get(components, smartlist_len(components) - 1); + last_label = (char *)smartlist_get(components, + smartlist_len(components) - 1); has_trailing_dot = (last_label[0] == '\0'); if (has_trailing_dot) { smartlist_pop_last(components); @@ -1133,12 +1134,20 @@ string_is_valid_hostname(const char *string) break; } - if (c_sl_idx == c_sl_len - 1) { + if (c_sl_idx == c_sl_len - 1) { // TLD validation. + int is_punycode = (strlen(c) > 4 && + (c[0] == 'X' || c[0] == 'x') && + (c[1] == 'N' || c[1] == 'n') && + c[2] == '-' && c[3] == '-'); + + if (is_punycode) + c += 4; + do { - result = TOR_ISALPHA(*c); + result = is_punycode ? TOR_ISALNUM(*c) : TOR_ISALPHA(*c); c++; } while (result && *c); - } else { + } else { // Regular hostname label validation. do { result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_')); c++; diff --git a/src/test/test_util.c b/src/test/test_util.c index db2ea1a34..ef1f420fe 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5595,6 +5595,10 @@ test_util_hostname_validation(void *arg) tt_assert(!string_is_valid_hostname("luck.y13")); tt_assert(!string_is_valid_hostname("luck.y13.")); + // We allow punycode TLDs. For examples, see + // http://data.iana.org/TLD/tlds-alpha-by-domain.txt + tt_assert(string_is_valid_hostname("example.xn--l1acc")); + done: return; }

1 0