From a23a705121ddd238e11ea329e2310290bb8046cc Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 27 Aug 2014 23:36:36 +0900 Subject: [PATCH] nghttpx: Strict integer config validation --- src/shrpx_config.cc | 278 ++++++++++++++++++++++++-------------------- 1 file changed, 154 insertions(+), 124 deletions(-) diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index b523c4c8..623f455e 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -297,6 +297,60 @@ std::pair parse_header(const char *optarg) return {std::string(optarg, colon), std::string(value, strlen(value))}; } +template +int parse_uint(T *dest, const char *opt, const char *optarg) +{ + char *end = nullptr; + + errno = 0; + + auto val = strtol(optarg, &end, 10); + + if(!optarg[0] || errno != 0 || *end || val < 0) { + LOG(ERROR) << opt << ": bad value. Specify an integer >= 0."; + return -1; + } + + *dest = val; + + return 0; +} + +template +int parse_int(T *dest, const char *opt, const char *optarg) +{ + char *end = nullptr; + + errno = 0; + + auto val = strtol(optarg, &end, 10); + + if(!optarg[0] || errno != 0 || *end) { + LOG(ERROR) << opt << ": bad value. Specify an integer."; + return -1; + } + + *dest = val; + + return 0; +} + +namespace { +int parse_timeval(timeval *dest, const char *opt, const char *optarg) +{ + time_t sec; + + if(parse_uint(&sec, opt, optarg) != 0) { + return -1; + } + + dest->tv_sec = sec; + dest->tv_usec = 0; + + return 0; +} +} // namespace + int parse_config(const char *opt, const char *optarg) { char host[NI_MAXHOST]; @@ -324,20 +378,17 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_WORKERS)) { - mod_config()->num_worker = strtol(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->num_worker, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_HTTP2_MAX_CONCURRENT_STREAMS)) { - mod_config()->http2_max_concurrent_streams = strtol(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->http2_max_concurrent_streams, + opt, optarg); } if(util::strieq(opt, SHRPX_OPT_LOG_LEVEL)) { if(Log::set_severity_level_by_name(optarg) == -1) { - LOG(ERROR) << "Invalid severity level: " << optarg; + LOG(ERROR) << opt << ": Invalid severity level: " << optarg; return -1; } @@ -381,52 +432,32 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_READ_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->http2_upstream_read_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->http2_upstream_read_timeout, + opt, optarg); } if(util::strieq(opt, SHRPX_OPT_FRONTEND_READ_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->upstream_read_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->upstream_read_timeout, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_FRONTEND_WRITE_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->upstream_write_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->upstream_write_timeout, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_BACKEND_READ_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->downstream_read_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->downstream_read_timeout, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_BACKEND_WRITE_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->downstream_write_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->downstream_write_timeout, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_STREAM_READ_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->stream_read_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->stream_read_timeout, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_STREAM_WRITE_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->stream_write_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->stream_write_timeout, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_ACCESSLOG_FILE)) { @@ -454,57 +485,67 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_BACKEND_KEEP_ALIVE_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->downstream_idle_read_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->downstream_idle_read_timeout, + opt, optarg); } if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_WINDOW_BITS) || util::strieq(opt, SHRPX_OPT_BACKEND_HTTP2_WINDOW_BITS)) { + size_t *resp; - const char *optname; + if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_WINDOW_BITS)) { resp = &mod_config()->http2_upstream_window_bits; - optname = SHRPX_OPT_FRONTEND_HTTP2_WINDOW_BITS; } else { resp = &mod_config()->http2_downstream_window_bits; - optname = SHRPX_OPT_BACKEND_HTTP2_WINDOW_BITS; } + errno = 0; - unsigned long int n = strtoul(optarg, nullptr, 10); - if(errno == 0 && n < 31) { - *resp = n; - } else { - LOG(ERROR) << "--" << optname - << " specify the integer in the range [0, 30], inclusive"; + + int n; + + if(parse_uint(&n, opt, optarg) != 0) { return -1; } + if(n >= 31) { + LOG(ERROR) << opt + << ": specify the integer in the range [0, 30], inclusive"; + return -1; + } + + *resp = n; + return 0; } if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_CONNECTION_WINDOW_BITS) || util::strieq(opt, SHRPX_OPT_BACKEND_HTTP2_CONNECTION_WINDOW_BITS)) { + size_t *resp; - const char *optname; + if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_CONNECTION_WINDOW_BITS)) { resp = &mod_config()->http2_upstream_connection_window_bits; - optname = SHRPX_OPT_FRONTEND_HTTP2_CONNECTION_WINDOW_BITS; } else { resp = &mod_config()->http2_downstream_connection_window_bits; - optname = SHRPX_OPT_BACKEND_HTTP2_CONNECTION_WINDOW_BITS; } + errno = 0; - unsigned long int n = strtoul(optarg, 0, 10); - if(errno == 0 && n >= 16 && n < 31) { - *resp = n; - } else { - LOG(ERROR) << "--" << optname - << " specify the integer in the range [16, 30], inclusive"; + + int n; + + if(parse_uint(&n, opt, optarg) != 0) { return -1; } + if(n < 16 || n >= 31) { + LOG(ERROR) << opt + << ": specify the integer in the range [16, 30], inclusive"; + return -1; + } + + *resp = n; + return 0; } @@ -535,7 +576,7 @@ int parse_config(const char *opt, const char *optarg) if(util::strieq(opt, SHRPX_OPT_USER)) { auto pwd = getpwnam(optarg); if(!pwd) { - LOG(ERROR) << "--user: failed to get uid from " << optarg + LOG(ERROR) << opt << ": failed to get uid from " << optarg << ": " << strerror(errno); return -1; } @@ -554,7 +595,7 @@ int parse_config(const char *opt, const char *optarg) if(util::strieq(opt, SHRPX_OPT_PRIVATE_KEY_PASSWD_FILE)) { auto passwd = read_passwd_from_file(optarg); if (passwd.empty()) { - LOG(ERROR) << "Couldn't read key file's passwd from " << optarg; + LOG(ERROR) << opt << ": Couldn't read key file's passwd from " << optarg; return -1; } mod_config()->private_key_passwd = strcopy(passwd); @@ -589,7 +630,7 @@ int parse_config(const char *opt, const char *optarg) if(util::strieq(opt, SHRPX_OPT_SYSLOG_FACILITY)) { int facility = int_syslog_facility(optarg); if(facility == -1) { - LOG(ERROR) << "Unknown syslog facility: " << optarg; + LOG(ERROR) << opt << ": Unknown syslog facility: " << optarg; return -1; } mod_config()->syslog_facility = facility; @@ -598,7 +639,18 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_BACKLOG)) { - mod_config()->backlog = strtol(optarg, nullptr, 10); + int n; + if(parse_int(&n, opt, optarg) != 0) { + return -1; + } + + if(n < -1) { + LOG(ERROR) << opt << ": " << optarg << " is not allowed"; + + return -1; + } + + mod_config()->backlog = n; return 0; } @@ -659,36 +711,36 @@ int parse_config(const char *opt, const char *optarg) http2::copy_url_component(val, &u, UF_HOST, optarg); mod_config()->downstream_http_proxy_host = strcopy(val); } else { - LOG(ERROR) << "backend-http-proxy-uri does not contain hostname"; + LOG(ERROR) << opt << ": no hostname specified"; return -1; } if(u.field_set & UF_PORT) { mod_config()->downstream_http_proxy_port = u.port; } else { - LOG(ERROR) << "backend-http-proxy-uri does not contain port"; + LOG(ERROR) << opt << ": no port specified"; return -1; } } else { - LOG(ERROR) << "Could not parse backend-http-proxy-uri"; - return -1; + LOG(ERROR) << opt << ": parse error"; + return -1; } return 0; } if(util::strieq(opt, SHRPX_OPT_READ_RATE)) { - mod_config()->read_rate = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->read_rate, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_READ_BURST)) { - errno = 0; - auto n = strtoul(optarg, nullptr, 10); + int n; - if(errno != 0 || n == 0) { - LOG(ERROR) << SHRPX_OPT_READ_BURST - << ": specify integer strictly larger than 0"; + if(parse_uint(&n, opt, optarg) != 0) { + return -1; + } + + if(n == 0) { + LOG(ERROR) << opt << ": specify integer strictly larger than 0"; return -1; } @@ -699,39 +751,27 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_WRITE_RATE)) { - mod_config()->write_rate = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->write_rate, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_WRITE_BURST)) { - mod_config()->write_burst = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->write_burst, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_WORKER_READ_RATE)) { - mod_config()->worker_read_rate = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->worker_read_rate, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_WORKER_READ_BURST)) { - mod_config()->worker_read_burst = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->worker_read_burst, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_RATE)) { - mod_config()->worker_write_rate = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->worker_write_rate, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_BURST)) { - mod_config()->worker_write_burst = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->worker_write_burst, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_NPN_LIST)) { @@ -799,9 +839,7 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_PADDING)) { - mod_config()->padding = strtoul(optarg, nullptr, 10); - - return 0; + return parse_uint(&mod_config()->padding, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_ALTSVC)) { @@ -809,21 +847,25 @@ int parse_config(const char *opt, const char *optarg) if(tokens.size() < 2) { // Requires at least protocol_id and port - LOG(ERROR) << "altsvc: too few parameters: " << optarg; + LOG(ERROR) << opt << ": too few parameters: " << optarg; return -1; } if(tokens.size() > 4) { // We only need protocol_id, port, host and origin - LOG(ERROR) << "altsvc: too many parameters: " << optarg; + LOG(ERROR) << opt << ": too many parameters: " << optarg; return -1; } - errno = 0; - auto port = strtoul(tokens[1], nullptr, 10); + int port; - if(errno != 0 || port < 1 || port > std::numeric_limits::max()) { - LOG(ERROR) << "altsvc: port is invalid: " << tokens[1]; + if(parse_uint(&port, opt, tokens[1]) != 0) { + return -1; + } + + if(port < 1 || + port > static_cast(std::numeric_limits::max())) { + LOG(ERROR) << opt << ": port is invalid: " << tokens[1]; return -1; } @@ -852,8 +894,7 @@ int parse_config(const char *opt, const char *optarg) if(util::strieq(opt, SHRPX_OPT_ADD_RESPONSE_HEADER)) { auto p = parse_header(optarg); if(p.first.empty()) { - LOG(ERROR) << "add-response-header: header field name is empty: " - << optarg; + LOG(ERROR) << opt << ": header field name is empty: " << optarg; return -1; } mod_config()->add_response_headers.push_back(std::move(p)); @@ -862,18 +903,7 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_WORKER_FRONTEND_CONNECTIONS)) { - errno = 0; - auto n = strtoul(optarg, nullptr, 10); - - if(errno != 0) { - LOG(ERROR) << "worker-frontend-connections: invalid argument: " - << optarg; - return -1; - } - - mod_config()->worker_frontend_connections = n; - - return 0; + return parse_uint(&mod_config()->worker_frontend_connections, opt, optarg); } if(util::strieq(opt, SHRPX_OPT_NO_LOCATION_REWRITE)) { @@ -883,13 +913,15 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_BACKEND_CONNECTIONS_PER_FRONTEND)) { - errno = 0; + int n; - auto n = strtoul(optarg, nullptr, 10); + if(parse_uint(&n, opt, optarg) != 0) { + return -1; + } + + if(n < 1) { + LOG(ERROR) << opt << ": specify the integer more than or equal to 1"; - if(errno != 0 || n < 1) { - LOG(ERROR) << "backend-connections-per-frontend: " - << "specify the integer more than or equal to 1"; return -1; } @@ -899,14 +931,12 @@ int parse_config(const char *opt, const char *optarg) } if(util::strieq(opt, SHRPX_OPT_LISTENER_DISABLE_TIMEOUT)) { - timeval tv = {strtol(optarg, nullptr, 10), 0}; - mod_config()->listener_disable_timeout = tv; - - return 0; + return parse_timeval(&mod_config()->listener_disable_timeout, + opt, optarg); } if(util::strieq(opt, "conf")) { - LOG(WARNING) << "conf is ignored"; + LOG(WARNING) << "conf: ignored"; return 0; }