nghttpx: Strict integer config validation

src/shrpx_config.cc

@@ -297,6 +297,60 @@ std::pair<std::string, std::string> parse_header(const char *optarg)
   return {std::string(optarg, colon), std::string(value, strlen(value))};
 }
 
+template<typename T>
+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<typename T>
+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 parse_uint(&mod_config()->num_worker, opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_HTTP2_MAX_CONCURRENT_STREAMS)) {
-    mod_config()->http2_max_concurrent_streams = strtol(optarg, nullptr, 10);
+    return parse_uint(&mod_config()->http2_max_concurrent_streams,
-
+                      opt, optarg);
-    return 0;
   }
 
   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};
+    return parse_timeval(&mod_config()->http2_upstream_read_timeout,
-    mod_config()->http2_upstream_read_timeout = tv;
+                         opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_FRONTEND_READ_TIMEOUT)) {
-    timeval tv = {strtol(optarg, nullptr, 10), 0};
+    return parse_timeval(&mod_config()->upstream_read_timeout, opt, optarg);
-    mod_config()->upstream_read_timeout = tv;
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_FRONTEND_WRITE_TIMEOUT)) {
-    timeval tv = {strtol(optarg, nullptr, 10), 0};
+    return parse_timeval(&mod_config()->upstream_write_timeout, opt, optarg);
-    mod_config()->upstream_write_timeout = tv;
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_BACKEND_READ_TIMEOUT)) {
-    timeval tv = {strtol(optarg, nullptr, 10), 0};
+    return parse_timeval(&mod_config()->downstream_read_timeout, opt, optarg);
-    mod_config()->downstream_read_timeout = tv;
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_BACKEND_WRITE_TIMEOUT)) {
-    timeval tv = {strtol(optarg, nullptr, 10), 0};
+    return parse_timeval(&mod_config()->downstream_write_timeout, opt, optarg);
-    mod_config()->downstream_write_timeout = tv;
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_STREAM_READ_TIMEOUT)) {
-    timeval tv = {strtol(optarg, nullptr, 10), 0};
+    return parse_timeval(&mod_config()->stream_read_timeout, opt, optarg);
-    mod_config()->stream_read_timeout = tv;
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_STREAM_WRITE_TIMEOUT)) {
-    timeval tv = {strtol(optarg, nullptr, 10), 0};
+    return parse_timeval(&mod_config()->stream_write_timeout, opt, optarg);
-    mod_config()->stream_write_timeout = tv;
-
-    return 0;
   }
 
   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};
+    return parse_timeval(&mod_config()->downstream_idle_read_timeout,
-    mod_config()->downstream_idle_read_timeout = tv;
+                         opt, optarg);
-
-    return 0;
   }
 
   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) {
+    int n;
-      *resp = n;
+
-    } else {
+    if(parse_uint(&n, opt, optarg) != 0) {
-      LOG(ERROR) << "--" << optname
-                 << " specify the integer in the range [0, 30], inclusive";
       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) {
+    int n;
-      *resp = n;
+
-    } else {
+    if(parse_uint(&n, opt, optarg) != 0) {
-      LOG(ERROR) << "--" << optname
-                 << " specify the integer in the range [16, 30], inclusive";
       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";
+      LOG(ERROR) << opt << ": parse error";
-        return -1;
+      return -1;
     }
 
     return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_READ_RATE)) {
-    mod_config()->read_rate = strtoul(optarg, nullptr, 10);
+    return parse_uint(&mod_config()->read_rate, opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_READ_BURST)) {
-    errno = 0;
+    int n;
-    auto n = strtoul(optarg, nullptr, 10);
 
-    if(errno != 0 || n == 0) {
+    if(parse_uint(&n, opt, optarg) != 0) {
-      LOG(ERROR) << SHRPX_OPT_READ_BURST
+      return -1;
-                 << ": specify integer strictly larger than 0";
+    }
+
+    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 parse_uint(&mod_config()->write_rate, opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_WRITE_BURST)) {
-    mod_config()->write_burst = strtoul(optarg, nullptr, 10);
+    return parse_uint(&mod_config()->write_burst, opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_WORKER_READ_RATE)) {
-    mod_config()->worker_read_rate = strtoul(optarg, nullptr, 10);
+    return parse_uint(&mod_config()->worker_read_rate, opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_WORKER_READ_BURST)) {
-    mod_config()->worker_read_burst = strtoul(optarg, nullptr, 10);
+    return parse_uint(&mod_config()->worker_read_burst, opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_RATE)) {
-    mod_config()->worker_write_rate = strtoul(optarg, nullptr, 10);
+    return parse_uint(&mod_config()->worker_write_rate, opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_BURST)) {
-    mod_config()->worker_write_burst = strtoul(optarg, nullptr, 10);
+    return parse_uint(&mod_config()->worker_write_burst, opt, optarg);
-
-    return 0;
   }
 
   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 parse_uint(&mod_config()->padding, opt, optarg);
-
-    return 0;
   }
 
   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;
+    int port;
-    auto port = strtoul(tokens[1], nullptr, 10);
 
-    if(errno != 0 || port < 1 || port > std::numeric_limits<uint16_t>::max()) {
+    if(parse_uint(&port, opt, tokens[1]) != 0) {
-      LOG(ERROR) << "altsvc: port is invalid: " << tokens[1];
+      return -1;
+    }
+
+    if(port < 1 ||
+       port > static_cast<int>(std::numeric_limits<uint16_t>::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: "
+      LOG(ERROR) << opt << ": header field name is empty: " << optarg;
-                 << 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;
+    return parse_uint(&mod_config()->worker_frontend_connections, opt, optarg);
-    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;
   }
 
   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};
+    return parse_timeval(&mod_config()->listener_disable_timeout,
-    mod_config()->listener_disable_timeout = tv;
+                         opt, optarg);
-
-    return 0;
   }
 
   if(util::strieq(opt, "conf")) {
-    LOG(WARNING) << "conf is ignored";
+    LOG(WARNING) << "conf: ignored";
 
     return 0;
   }