From 10dbe38a92cc278170213a6f50b0d3d5288113ac Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Tue, 19 May 2020 03:54:03 -0400 Subject: [PATCH] [core] stricter parse of numerical digits stricter parse of numerical digits for http status code, port num, and a few other places. (stricter parse than that of strtol()) content ranges are still parsed more loosely at points of use --- src/http-header-glue.c | 8 ++++---- src/http_header.c | 9 +++++++++ src/http_header.h | 3 +++ src/mod_extforward.c | 27 ++++++++++++++++++++------- src/mod_magnet.c | 5 ++--- src/mod_wstunnel.c | 6 ++++-- 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/http-header-glue.c b/src/http-header-glue.c index cf77ae1a..addf306b 100644 --- a/src/http-header-glue.c +++ b/src/http-header-glue.c @@ -950,7 +950,7 @@ static int http_response_process_headers(request_st * const r, http_response_opt /* non-parsed headers ... we parse them anyway */ if ((s[7] == '1' || s[7] == '0') && s[8] == ' ') { /* after the space should be a status code for us */ - int status = strtol(s+9, NULL, 10); + int status = http_header_str_to_code(s+9); if (status >= 100 && status < 1000) { status_is_set = 1; r->resp_htags |= HTTP_HEADER_STATUS; @@ -983,7 +983,7 @@ static int http_response_process_headers(request_st * const r, http_response_opt if (opts->authorizer) { if (0 == r->http_status || 200 == r->http_status) { if (id == HTTP_HEADER_STATUS) { - int status = strtol(value, NULL, 10); + int status = http_header_str_to_code(value); if (status >= 100 && status < 1000) { r->http_status = status; } else { @@ -1004,9 +1004,8 @@ static int http_response_process_headers(request_st * const r, http_response_opt switch (id) { case HTTP_HEADER_STATUS: { - int status; if (opts->backend == BACKEND_PROXY) break; /*(pass w/o parse)*/ - status = strtol(value, NULL, 10); + int status = http_header_str_to_code(value); if (status >= 100 && status < 1000) { r->http_status = status; status_is_set = 1; @@ -1034,6 +1033,7 @@ static int http_response_process_headers(request_st * const r, http_response_opt break; case HTTP_HEADER_CONTENT_LENGTH: r->content_length = strtoul(value, NULL, 10); + if (*value == '+') ++value; break; case HTTP_HEADER_TRANSFER_ENCODING: break; diff --git a/src/http_header.c b/src/http_header.c index f462c5ba..897a0fd3 100644 --- a/src/http_header.c +++ b/src/http_header.c @@ -63,6 +63,15 @@ enum http_header_e http_header_hkey_get(const char * const s, const uint32_t sle } +int http_header_str_to_code (const char * const s) +{ + /*(more strict than strtol(); exactly 3 digits followed by SP/TAB/NIL)*/ + return (light_isdigit(s[0]) && light_isdigit(s[1]) && light_isdigit(s[2]) + && (s[3] == '\0' || s[3] == ' ' || s[3] == '\t')) + ? (s[0]-'0')*100 + (s[1]-'0')*10 + (s[2]-'0') + : -1; +} + int http_header_str_contains_token (const char * const s, const uint32_t slen, const char * const m, const uint32_t mlen) { /*if (slen < mlen) return 0;*//*(possible optimizations for caller)*/ diff --git a/src/http_header.h b/src/http_header.h index 6cd1629a..72737ef5 100644 --- a/src/http_header.h +++ b/src/http_header.h @@ -43,6 +43,9 @@ enum http_header_e { __attribute_pure__ enum http_header_e http_header_hkey_get(const char *s, uint32_t slen); +__attribute_pure__ +int http_header_str_to_code (const char * const s); + __attribute_pure__ int http_header_str_contains_token (const char *s, uint32_t slen, const char *m, uint32_t mlen); diff --git a/src/mod_extforward.c b/src/mod_extforward.c index b8e16773..84d54e1f 100644 --- a/src/mod_extforward.c +++ b/src/mod_extforward.c @@ -248,7 +248,7 @@ static void * mod_extforward_parse_forwarder(server *srv, const array *forwarder char *err; const int nm_bits = strtol(nm_slash + 1, &err, 10); int rc; - if (*err || nm_bits <= 0) { + if (*err || nm_bits <= 0 || !light_isdigit(nm_slash[1])) { log_error(srv->errh, __FILE__, __LINE__, "ERROR: invalid netmask: %s %s", ds->key.ptr, err); free(fwd); @@ -1398,6 +1398,19 @@ static int hap_PROXY_recv (const int fd, union hap_PROXY_hdr * const hdr, const } +__attribute_pure__ +static int mod_extforward_str_to_port (const char * const s) +{ + /*(more strict than strtol(); digits only)*/ + int port = 0; + for (int i = 0; i < 5; ++i, port *= 10) { + if (!light_isdigit(s[i])) return -1; + port += (s[i] - '0'); + if (s[i+1] == '\0') return port; + } + return -1; +} + static int mod_extforward_hap_PROXY_v1 (connection * const con, union hap_PROXY_hdr * const hdr) { @@ -1412,9 +1425,9 @@ static int mod_extforward_hap_PROXY_v1 (connection * const con, * "PROXY UNKNOWN ffff:f...f:ffff ffff:f...f:ffff 65535 65535\r\n" */ char *s = hdr->v1.line + sizeof("PROXY")-1; /*checked in hap_PROXY_recv()*/ - char *src_addr, *dst_addr, *src_port, *dst_port, *e; + char *src_addr, *dst_addr, *src_port, *dst_port; int family; - long src_lport, dst_lport; + int src_lport, dst_lport; if (*s != ' ') return -1; ++s; if (s[0] == 'T' && s[1] == 'C' && s[2] == 'P' && s[4] == ' ') { @@ -1448,10 +1461,10 @@ static int mod_extforward_hap_PROXY_v1 (connection * const con, if (NULL == dst_port) return -1; *dst_port++ = '\0'; - src_lport = strtol(src_port, &e, 10); - if (src_lport <= 0 || src_lport > USHRT_MAX || *e != '\0') return -1; - dst_lport = strtol(dst_port, &e, 10); - if (dst_lport <= 0 || dst_lport > USHRT_MAX || *e != '\0') return -1; + src_lport = mod_extforward_str_to_port(src_port); + if (src_lport <= 0) return -1; + dst_lport = mod_extforward_str_to_port(dst_port); + if (dst_lport <= 0) return -1; if (1 != sock_addr_inet_pton(&con->dst_addr, src_addr, family, (unsigned short)src_lport)) diff --git a/src/mod_magnet.c b/src/mod_magnet.c index 6e80bd7e..4521ec16 100644 --- a/src/mod_magnet.c +++ b/src/mod_magnet.c @@ -1031,10 +1031,9 @@ static handler_t magnet_attract_array(request_st * const r, plugin_data * const if (r->error_handler_saved_status) { /* retrieve (possibly modified) REDIRECT_STATUS and store as number */ - unsigned long x; + int x; const buffer * const vb = http_header_env_get(r, CONST_STR_LEN("REDIRECT_STATUS")); - if (vb && (x = strtoul(vb->ptr, NULL, 10)) < 1000) - /*(simplified validity check x < 1000)*/ + if (vb && (x = http_header_str_to_code(vb->ptr)) != -1) r->error_handler_saved_status = r->error_handler_saved_status > 0 ? (int)x : -(int)x; } diff --git a/src/mod_wstunnel.c b/src/mod_wstunnel.c index 934f13cb..389cd7b4 100644 --- a/src/mod_wstunnel.c +++ b/src/mod_wstunnel.c @@ -443,7 +443,9 @@ static int wstunnel_is_allowed_origin(request_st * const r, handler_ctx * const static int wstunnel_check_request(request_st * const r, handler_ctx * const hctx) { const buffer * const vers = http_header_request_get(r, HTTP_HEADER_OTHER, CONST_STR_LEN("Sec-WebSocket-Version")); - const long hybivers = (NULL != vers) ? strtol(vers->ptr, NULL, 10) : 0; + const long hybivers = (NULL != vers) + ? light_isdigit(*vers->ptr) ? strtol(vers->ptr, NULL, 10) : -1 + : 0; if (hybivers < 0 || hybivers > INT_MAX) { DEBUG_LOG_ERR("%s", "invalid Sec-WebSocket-Version"); r->http_status = 400; /* Bad Request */ @@ -689,7 +691,7 @@ static int get_key_number(uint32_t *ret, const buffer *b) { } tmp[j] = '\0'; n = strtoul(tmp, NULL, 10); - if (n > UINT32_MAX || 0 == sp) return -1; + if (n > UINT32_MAX || 0 == sp || !light_isdigit(*tmp)) return -1; *ret = (uint32_t)n / sp; return 0; }