From b0ff1fd270924c5eaec09687e3d279130123671a Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 27 Oct 2022 13:54:27 +0200 Subject: [PATCH 1/2] noproxy: also match with adjacent comma If the host name is an IP address and the noproxy string contained that IP address with a following comma, it would erroneously not match. Extended test 1614 to verify this combo as well. Reported-by: Henning Schild Fixes #9813 Closes #9814 Upstream-commit: efc286b7a62af0568fdcbf3c68791c9955182128 Signed-off-by: Kamil Dudka --- lib/noproxy.c | 20 ++++++++++++-------- tests/data/test1614 | 2 +- tests/unit/unit1614.c | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/lib/noproxy.c b/lib/noproxy.c index 81f1e09..d08a16b 100644 --- a/lib/noproxy.c +++ b/lib/noproxy.c @@ -188,18 +188,22 @@ bool Curl_check_noproxy(const char *name, const char *no_proxy) /* FALLTHROUGH */ case TYPE_IPV6: { const char *check = token; - char *slash = strchr(check, '/'); + char *slash; unsigned int bits = 0; char checkip[128]; + if(tokenlen >= sizeof(checkip)) + /* this cannot match */ + break; + /* copy the check name to a temp buffer */ + memcpy(checkip, check, tokenlen); + checkip[tokenlen] = 0; + check = checkip; + + slash = strchr(check, '/'); /* if the slash is part of this token, use it */ - if(slash && (slash < &check[tokenlen])) { + if(slash) { bits = atoi(slash + 1); - /* copy the check name to a temp buffer */ - if(tokenlen >= sizeof(checkip)) - break; - memcpy(checkip, check, tokenlen); - checkip[ slash - check ] = 0; - check = checkip; + *slash = 0; /* null terminate there */ } if(type == TYPE_IPV6) match = Curl_cidr6_match(name, check, bits); diff --git a/tests/data/test1614 b/tests/data/test1614 index 4a9d54e..73bdbb4 100644 --- a/tests/data/test1614 +++ b/tests/data/test1614 @@ -16,7 +16,7 @@ unittest proxy -cidr comparisons +noproxy and cidr comparisons diff --git a/tests/unit/unit1614.c b/tests/unit/unit1614.c index 6028545..c2f563a 100644 --- a/tests/unit/unit1614.c +++ b/tests/unit/unit1614.c @@ -77,6 +77,20 @@ UNITTEST_START { NULL, NULL, 0, FALSE} /* end marker */ }; struct noproxy list[]= { + { "127.0.0.1", "127.0.0.1,localhost", TRUE}, + { "127.0.0.1", "127.0.0.1,localhost,", TRUE}, + { "127.0.0.1", "127.0.0.1/8,localhost,", TRUE}, + { "127.0.0.1", "127.0.0.1/28,localhost,", TRUE}, + { "127.0.0.1", "127.0.0.1/31,localhost,", TRUE}, + { "127.0.0.1", "localhost,127.0.0.1", TRUE}, + { "127.0.0.1", "localhost,127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1." + "127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1.127." + "0.0.1.127.0.0.1.127.0.0." /* 128 bytes "address" */, FALSE}, + { "127.0.0.1", "localhost,127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1." + "127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1.127.0.0.1.127." + "0.0.1.127.0.0.1.127.0.0" /* 127 bytes "address" */, FALSE}, + { "localhost", "localhost,127.0.0.1", TRUE}, + { "localhost", "127.0.0.1,localhost", TRUE}, { "foobar", "barfoo", FALSE}, { "foobar", "foobar", TRUE}, { "192.168.0.1", "foobar", FALSE}, -- 2.37.3 From d539fd9f11e2a244dbab6b9171f5a9e5c86cc417 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 28 Oct 2022 10:51:49 +0200 Subject: [PATCH 2/2] noproxy: fix tail-matching Also ignore trailing dots in both host name and comparison pattern. Regression in 7.86.0 (from 1e9a538e05c0) Extended test 1614 to verify better. Reported-by: Henning Schild Fixes #9821 Closes #9822 Upstream-commit: b830f9ba9e94acf672cd191993ff679fa888838b Signed-off-by: Kamil Dudka --- lib/noproxy.c | 30 +++++++++++++++++++++++------- tests/unit/unit1614.c | 9 +++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/lib/noproxy.c b/lib/noproxy.c index d08a16b..01f8f47 100644 --- a/lib/noproxy.c +++ b/lib/noproxy.c @@ -149,9 +149,14 @@ bool Curl_check_noproxy(const char *name, const char *no_proxy) } else { unsigned int address; + namelen = strlen(name); if(1 == Curl_inet_pton(AF_INET, name, &address)) type = TYPE_IPV4; - namelen = strlen(name); + else { + /* ignore trailing dots in the host name */ + if(name[namelen - 1] == '.') + namelen--; + } } while(*p) { @@ -173,12 +178,23 @@ bool Curl_check_noproxy(const char *name, const char *no_proxy) if(tokenlen) { switch(type) { case TYPE_HOST: - if(*token == '.') { - ++token; - --tokenlen; - /* tailmatch */ - match = (tokenlen <= namelen) && - strncasecompare(token, name + (namelen - tokenlen), namelen); + /* ignore trailing dots in the token to check */ + if(token[tokenlen - 1] == '.') + tokenlen--; + + if(tokenlen && (*token == '.')) { + /* A: example.com matches '.example.com' + B: www.example.com matches '.example.com' + C: nonexample.com DOES NOT match '.example.com' + */ + if((tokenlen - 1) == namelen) + /* case A, exact match without leading dot */ + match = strncasecompare(token + 1, name, namelen); + else if(tokenlen < namelen) + /* case B, tailmatch with leading dot */ + match = strncasecompare(token, name + (namelen - tokenlen), + tokenlen); + /* case C passes through, not a match */ } else match = (tokenlen == namelen) && diff --git a/tests/unit/unit1614.c b/tests/unit/unit1614.c index c2f563a..8f62b70 100644 --- a/tests/unit/unit1614.c +++ b/tests/unit/unit1614.c @@ -77,6 +77,15 @@ UNITTEST_START { NULL, NULL, 0, FALSE} /* end marker */ }; struct noproxy list[]= { + { "www.example.com", "localhost,.example.com,.example.de", TRUE}, + { "www.example.com.", "localhost,.example.com,.example.de", TRUE}, + { "example.com", "localhost,.example.com,.example.de", TRUE}, + { "example.com.", "localhost,.example.com,.example.de", TRUE}, + { "www.example.com", "localhost,.example.com.,.example.de", TRUE}, + { "www.example.com", "localhost,www.example.com.,.example.de", TRUE}, + { "example.com", "localhost,example.com,.example.de", TRUE}, + { "example.com.", "localhost,example.com,.example.de", TRUE}, + { "www.example.com", "localhost,example.com,.example.de", FALSE}, { "127.0.0.1", "127.0.0.1,localhost", TRUE}, { "127.0.0.1", "127.0.0.1,localhost,", TRUE}, { "127.0.0.1", "127.0.0.1/8,localhost,", TRUE}, -- 2.37.3