From a3fb54670832f8e0ff8f69d9502e2fcf710da39d Mon Sep 17 00:00:00 2001 From: dmiller Date: Tue, 20 Sep 2022 23:37:29 +0000 Subject: [PATCH] Fix proxy parsing to fail on empty string. Fixes #177 --- nsock/src/nsock_proxy.c | 135 +++++++++++++++------------------------ nsock/tests/proxychain.c | 36 +++++++---- 2 files changed, 76 insertions(+), 95 deletions(-) diff --git a/nsock/src/nsock_proxy.c b/nsock/src/nsock_proxy.c index c99ddccf0..e2fd46ddf 100644 --- a/nsock/src/nsock_proxy.c +++ b/nsock/src/nsock_proxy.c @@ -60,18 +60,7 @@ #define IN_RANGE(x, min, max) ((x) >= (min) && (x) <= (max)) - -struct proxy_parser { - int done; - struct proxy_node *value; - char *str; - char *tokens; -}; - -static struct proxy_parser *proxy_parser_new(const char *proxychainstr); -static void proxy_parser_next(struct proxy_parser *parser); -static void proxy_parser_delete(struct proxy_parser *parser); - +static struct proxy_node *proxy_node_new(const char *proxystr, const char *end); /* --- Implemented proxy backends --- */ extern const struct proxy_spec ProxySpecHttp; @@ -97,18 +86,22 @@ int nsock_proxychain_new(const char *proxystr, nsock_proxychain *chain, nsock_po gh_list_init(&pxc->nodes); if (proxystr) { - struct proxy_parser *parser; + const char *next = proxystr; + const char *end = strchr(proxystr, ','); + struct proxy_node *proxy; - parser = proxy_parser_new(proxystr); - while (!parser->done) { - if (parser->value == NULL) { - nsock_proxychain_delete(pxc); + while (end != NULL) { + proxy = proxy_node_new(next, end); + if (!proxy) return -1; - } - gh_list_append(&pxc->nodes, &parser->value->nodeq); - proxy_parser_next(parser); + gh_list_append(&pxc->nodes, &proxy->nodeq); + next = end + 1; + end = strchr(next, ','); } - proxy_parser_delete(parser); + proxy = proxy_node_new(next, strchr(next, '\0')); + if (!proxy) + return -1; + gh_list_append(&pxc->nodes, &proxy->nodeq); } if (nsp) { @@ -149,6 +142,11 @@ int nsock_pool_set_proxychain(nsock_pool nspool, nsock_proxychain chain) { return -1; } + if (gh_list_count(&chain->nodes) < 1) { + nsock_log_error("Invalid call. No proxies in chain"); + return -1; + } + nsp->px_chain = (struct proxy_chain *)chain; return 1; } @@ -235,13 +233,13 @@ static int percent_decode(char *s) { return p - s; } -static int uri_parse_authority(const char *authority, struct uri *uri) { +static int uri_parse_authority(const char *authority, const char *end, struct uri *uri) { const char *portsep; const char *host_start, *host_end; const char *tail; /* We do not support "user:pass@" userinfo. The proxy has no use for it. */ - if (strchr(authority, '@') != NULL) + if (strchr_p(authority, end, '@') != NULL) return -1; /* Find the beginning and end of the host. */ @@ -250,32 +248,33 @@ static int uri_parse_authority(const char *authority, struct uri *uri) { if (*host_start == '[') { /* IPv6 address in brackets. */ host_start++; - host_end = strchr(host_start, ']'); - - if (host_end == NULL) + if (host_start >= end || + NULL == (host_end = strchr_p(host_start, end, ']')) + ) { + nsock_log_error("Invalid IPv6 address: %s", authority); return -1; + } portsep = host_end + 1; - if (!(*portsep == ':' || *portsep == '\0')) - return -1; - } else { - portsep = strrchr(authority, ':'); + for (portsep = end; portsep > host_start && *portsep != ':'; portsep--); - if (portsep == NULL) - portsep = strchr(authority, '\0'); + if (portsep == host_start) + portsep = end; host_end = portsep; } /* Get the port number. */ - if (*portsep == ':' && *(portsep + 1) != '\0') { + if (portsep + 1 < end && *portsep == ':') { long n; errno = 0; n = parse_long(portsep + 1, &tail); - if (errno || *tail || (tail == (portsep + 1)) || !IN_RANGE(n, 1, 65535)) + if (errno || tail != end || !IN_RANGE(n, 1, 65535)) { + nsock_log_error("Invalid port number (%ld), errno = %d", n, errno); return -1; + } uri->port = n; } else { uri->port = -1; @@ -284,6 +283,7 @@ static int uri_parse_authority(const char *authority, struct uri *uri) { /* Get the host. */ uri->host = mkstr(host_start, host_end); if (percent_decode(uri->host) < 0) { + nsock_log_error("Invalid URL encoding in host: %s", uri->host); free(uri->host); uri->host = NULL; return -1; @@ -292,7 +292,7 @@ static int uri_parse_authority(const char *authority, struct uri *uri) { return 1; } -static int parse_uri(const char *proxystr, struct uri *uri) { +static int parse_uri(const char *proxystr, const char *end, struct uri *uri) { const char *p, *q; /* Scheme, section 3.1. */ @@ -301,8 +301,11 @@ static int parse_uri(const char *proxystr, struct uri *uri) { goto fail; q = p; - while (isalpha(*q) || isdigit(*q) || *q == '+' || *q == '-' || *q == '.') + while (isalpha(*q) || isdigit(*q) || *q == '+' || *q == '-' || *q == '.') { q++; + if (q >= end) + goto fail; + } if (*q != ':') goto fail; @@ -316,20 +319,18 @@ static int parse_uri(const char *proxystr, struct uri *uri) { /* Authority, section 3.2. */ p = q + 1; - if (*p == '/' && *(p + 1) == '/') { - char *authority = NULL; + if (p >= end) + goto fail; + if (p + 1 < end && *p == '/' && *(p + 1) == '/') { p += 2; q = p; - while (!(*q == '/' || *q == '?' || *q == '#' || *q == '\0')) + while (q < end && !(*q == '/' || *q == '?' || *q == '#' || *q == '\0')) q++; - ; - authority = mkstr(p, q); - if (uri_parse_authority(authority, uri) < 0) { - free(authority); + + if (uri_parse_authority(p, q, uri) < 0) { goto fail; } - free(authority); p = q; } @@ -338,8 +339,7 @@ static int parse_uri(const char *proxystr, struct uri *uri) { * path is also not percent-decoded because we just pass it on to the origin * server. */ - q = strchr(p, '\0'); - uri->path = mkstr(p, q); + uri->path = mkstr(p, end); return 1; @@ -348,24 +348,27 @@ fail: return -1; } -static struct proxy_node *proxy_node_new(char *proxystr) { +static struct proxy_node *proxy_node_new(const char *proxystr, const char *end) { int i; for (i = 0; ProxyBackends[i] != NULL; i++) { const struct proxy_spec *pspec; + size_t prefix_len; pspec = ProxyBackends[i]; - if (strncasecmp(proxystr, pspec->prefix, strlen(pspec->prefix)) == 0) { + prefix_len = strlen(pspec->prefix); + if (end - proxystr > prefix_len && strncasecmp(proxystr, pspec->prefix, prefix_len) == 0) { struct proxy_node *proxy = NULL; struct uri uri; memset(&uri, 0x00, sizeof(struct uri)); - if (parse_uri(proxystr, &uri) < 0) + if (parse_uri(proxystr, end, &uri) < 0) break; if (pspec->ops->node_new(&proxy, &uri) < 0) { nsock_log_error("Cannot initialize proxy node %s", proxystr); + uri_free(&uri); break; } @@ -378,40 +381,6 @@ static struct proxy_node *proxy_node_new(char *proxystr) { return NULL; } -struct proxy_parser *proxy_parser_new(const char *proxychainstr) { - struct proxy_parser *parser; - - parser = (struct proxy_parser *)safe_malloc(sizeof(struct proxy_parser)); - parser->done = 0; - parser->value = NULL; - - parser->str = strdup(proxychainstr); - - parser->tokens = strtok(parser->str, ","); - if (parser->tokens) - parser->value = proxy_node_new(parser->tokens); - else - parser->done = 1; - - return parser; -} - -void proxy_parser_next(struct proxy_parser *parser) { - - parser->tokens = strtok(NULL, ","); - if (parser->tokens) - parser->value = proxy_node_new(parser->tokens); - else - parser->done = 1; -} - -void proxy_parser_delete(struct proxy_parser *parser) { - if (parser) { - free(parser->str); - free(parser); - } -} - void forward_event(nsock_pool nspool, nsock_event nsevent, void *udata) { struct npool *nsp = (struct npool *)nspool; struct nevent *nse = (struct nevent *)nsevent; diff --git a/nsock/tests/proxychain.c b/nsock/tests/proxychain.c index e738187fb..2360a6838 100644 --- a/nsock/tests/proxychain.c +++ b/nsock/tests/proxychain.c @@ -24,13 +24,15 @@ static const struct proxy_test Tests[] = { /* single proxy */ /* http */ {GOOD, "http://example.com"}, - {GOOD, "http://example.com/some/crazy.path"}, - {GOOD, "http://example.com/some/path?q=@!weird&other=;"}, {GOOD, "http://127.0.0.1/"}, + {GOOD, "http://1/some/crazy.path"}, + {GOOD, "http://127.1/some/path?q=@!weird&other=;"}, {GOOD, "http://[::1]/"}, - {GOOD, "http://example.com:80/"}, + {GOOD, "http://1:80/"}, + {GOOD, "http://1:8080"}, {GOOD, "http://127.0.0.1:1234/"}, {GOOD, "http://[::1]:8080/"}, + {GOOD, "http://[::1]:80"}, /* https not supported! */ {BAD, "https://example.com/"}, /* No username/password in URI */ @@ -43,6 +45,7 @@ static const struct proxy_test Tests[] = { {BAD, "http://:8080/"}, /* socks4 */ {GOOD, "socks4://example.com"}, + {GOOD, "socks4://1"}, /* socks4 does not support IPv6 */ {BAD, "socks4://[::1]"}, /* Does SOCKS4 really support a path like this? */ @@ -53,8 +56,6 @@ static const struct proxy_test Tests[] = { /* Should fail: socks4 cannot connect to IPv6 proxy */ {GOOD, "socks4://127.0.0.1/,socks4://example.com/,http://[::1]:9090"}, /* Dumb stuff */ -#if 0 - /* These tests should fail, but do not yet: #177 */ {BAD, ""}, {BAD, ","}, {BAD, ",,"}, @@ -65,14 +66,22 @@ static const struct proxy_test Tests[] = { {BAD, "socks4://127.0.0.1/,http://example.com/,"}, {BAD, ",socks4://127.0.0.1/,http://example.com/"}, {BAD, "socks4://127.0.0.1/,,http://example.com/"}, -#endif + {BAD, "http://example.com:-1/"}, + {BAD, "http://example.com:0x80/"}, + {BAD, "http://example.com:0/"}, + {BAD, "http://example.com:2147483648"}, + {BAD, "http://example.com:21474836480"}, + {BAD, "http://:80"}, + {BAD, "http://example.com:80.com"}, {BAD, "com"}, {BAD, "example.com"}, {BAD, "/example.com/"}, {BAD, "//example.com/"}, {BAD, "http/example.com/"}, {BAD, "http//example.com/"}, + {BAD, "http:///example.com"}, {BAD, "sptth://example.com/"}, + {BAD, " "}, {BAD, ", "}, {BAD, " ,"}, {BAD, ", ,"}, @@ -82,6 +91,7 @@ static const struct proxy_test Tests[] = { }; static int parser_test(void *testdata) { + int result = 0; struct proxy_test_data *ptd = (struct proxy_test_data *)testdata; const struct proxy_test *pt = &Tests[ptd->tn]; while (pt->ttype != END_OF_TESTS) { @@ -90,19 +100,21 @@ static int parser_test(void *testdata) { nsock_log_info("Expected failure:"); int ret = nsock_proxychain_new(pt->input, &pxc, NULL); nsock_log_debug("Test %d result: %d", ptd->tn, ret); - if (ret > 0 && pt->ttype == BAD) { - fprintf(stderr, "Proxy Test #%d: Failed to reject bad input: %s\n", ptd->tn, pt->input); - return -1; + if (ret > 0) { + if (pt->ttype == BAD) { + fprintf(stderr, "Proxy Test #%d: Failed to reject bad input: %s\n", ptd->tn, pt->input); + result = -1; + } + nsock_proxychain_delete(pxc); } else if (ret <= 0 && pt->ttype == GOOD) { fprintf(stderr, "Proxy Test #%d: Failed to parse good input: %s\n", ptd->tn, pt->input); - return -2; + result = -2; } - nsock_proxychain_delete(pxc); ptd->tn++; pt = &Tests[ptd->tn]; } - return 0; + return result; } static void log_handler(const struct nsock_log_rec *rec) {