4 files changed, 143 insertions, 135 deletions

diff --git a/monitor.c b/monitor.c
index f517da482..a0ad9857d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.174 2017/10/02 19:33:20 djm Exp $ */
+/* $OpenBSD: monitor.c,v 1.175 2017/10/05 15:52:03 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -760,12 +760,10 @@ mm_answer_pwnamallow(int sock, Buffer *m)
                 for (i = 0; i < options.nx; i++) \
                         buffer_put_cstring(m, options.x[i]); \
         } while (0)
-#define M_CP_STRARRAYOPT_ALLOC(x, nx) M_CP_STRARRAYOPT(x, nx)
         /* See comment in servconf.h */
         COPY_MATCH_STRING_OPTS();
 #undef M_CP_STROPT
 #undef M_CP_STRARRAYOPT
-#undef M_CP_STRARRAYOPT_ALLOC
 
         /* Create valid auth method lists */
         if (auth2_setup_methods_lists(authctxt) != 0) {
diff --git a/monitor_wrap.c b/monitor_wrap.c
index 69212aaf3..a46628fb1 100644
--- a/monitor_wrap.c
+++ b/monitor_wrap.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.c,v 1.94 2017/10/02 19:33:20 djm Exp $ */
+/* $OpenBSD: monitor_wrap.c,v 1.95 2017/10/05 15:52:03 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -287,19 +287,15 @@ out:
                         newopts->x = buffer_get_string(&m, NULL); \
         } while (0)
 #define M_CP_STRARRAYOPT(x, nx) do { \
-                for (i = 0; i < newopts->nx; i++) \
-                        newopts->x[i] = buffer_get_string(&m, NULL); \
-        } while (0)
-#define M_CP_STRARRAYOPT_ALLOC(x, nx) do { \
                 newopts->x = newopts->nx == 0 ? \
                     NULL : xcalloc(newopts->nx, sizeof(*newopts->x)); \
-                M_CP_STRARRAYOPT(x, nx); \
+                for (i = 0; i < newopts->nx; i++) \
+                        newopts->x[i] = buffer_get_string(&m, NULL); \
         } while (0)
         /* See comment in servconf.h */
         COPY_MATCH_STRING_OPTS();
 #undef M_CP_STROPT
 #undef M_CP_STRARRAYOPT
-#undef M_CP_STRARRAYOPT_ALLOC
 
         copy_set_server_options(&options, newopts, 1);
         log_change_level(options.log_level);
diff --git a/servconf.c b/servconf.c
index 956862959..a96df4f67 100644
--- a/servconf.c
+++ b/servconf.c
@@ -1,5 +1,5 @@
 
-/* $OpenBSD: servconf.c,v 1.313 2017/10/04 18:49:30 djm Exp $ */
+/* $OpenBSD: servconf.c,v 1.314 2017/10/05 15:52:03 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -188,10 +188,45 @@ assemble_algorithms(ServerOptions *o)
                 fatal("kex_assemble_names failed");
 }
 
+static void
+array_append(const char *file, const int line, const char *directive,
+    char ***array, u_int *lp, const char *s)
+{
+
+        if (*lp >= INT_MAX)
+                fatal("%s line %d: Too many %s entries", file, line, directive);
+
+        *array = xrecallocarray(*array, *lp, *lp + 1, sizeof(**array));
+        (*array)[*lp] = xstrdup(s);
+        (*lp)++;
+}
+
+void
+servconf_add_hostkey(const char *file, const int line,
+    ServerOptions *options, const char *path)
+{
+        char *apath = derelativise_path(path);
+
+        array_append(file, line, "HostKey",
+            &options->host_key_files, &options->num_host_key_files, apath);
+        free(apath);
+}
+
+void
+servconf_add_hostcert(const char *file, const int line,
+    ServerOptions *options, const char *path)
+{
+        char *apath = derelativise_path(path);
+
+        array_append(file, line, "HostCertificate",
+            &options->host_cert_files, &options->num_host_cert_files, apath);
+        free(apath);
+}
+
 void
 fill_default_server_options(ServerOptions *options)
 {
-        int i;
+        u_int i;
 
         /* Portable-specific options */
         if (options->use_pam == -1)
@@ -200,16 +235,16 @@ fill_default_server_options(ServerOptions *options)
         /* Standard Options */
         if (options->num_host_key_files == 0) {
                 /* fill default hostkeys for protocols */
-                options->host_key_files[options->num_host_key_files++] =
-                    _PATH_HOST_RSA_KEY_FILE;
-                options->host_key_files[options->num_host_key_files++] =
-                    _PATH_HOST_DSA_KEY_FILE;
+                servconf_add_hostkey("[default]", 0, options,
+                    _PATH_HOST_RSA_KEY_FILE);
+                servconf_add_hostkey("[default]", 0, options,
+                    _PATH_HOST_DSA_KEY_FILE);
 #ifdef OPENSSL_HAS_ECC
-                options->host_key_files[options->num_host_key_files++] =
-                    _PATH_HOST_ECDSA_KEY_FILE;
+                servconf_add_hostkey("[default]", 0, options,
+                    _PATH_HOST_ECDSA_KEY_FILE);
 #endif
-                options->host_key_files[options->num_host_key_files++] =
-                    _PATH_HOST_ED25519_KEY_FILE;
+                servconf_add_hostkey("[default]", 0, options,
+                    _PATH_HOST_ED25519_KEY_FILE);
         }
         /* No certificates by default */
         if (options->num_ports == 0)
@@ -313,10 +348,14 @@ fill_default_server_options(ServerOptions *options)
         if (options->client_alive_count_max == -1)
                 options->client_alive_count_max = 3;
         if (options->num_authkeys_files == 0) {
-                options->authorized_keys_files[options->num_authkeys_files++] =
-                    xstrdup(_PATH_SSH_USER_PERMITTED_KEYS);
-                options->authorized_keys_files[options->num_authkeys_files++] =
-                    xstrdup(_PATH_SSH_USER_PERMITTED_KEYS2);
+                array_append("[default]", 0, "AuthorizedKeysFiles",
+                    &options->authorized_keys_files,
+                    &options->num_authkeys_files,
+                    _PATH_SSH_USER_PERMITTED_KEYS);
+                array_append("[default]", 0, "AuthorizedKeysFiles",
+                    &options->authorized_keys_files,
+                    &options->num_authkeys_files,
+                    _PATH_SSH_USER_PERMITTED_KEYS2);
         }
         if (options->permit_tun == -1)
                 options->permit_tun = SSH_TUNMODE_NO;
@@ -1128,22 +1167,12 @@ process_server_config_line(ServerOptions *options, char *line,
                 break;
 
         case sHostKeyFile:
-                intptr = &options->num_host_key_files;
-                if (*intptr >= MAX_HOSTKEYS)
-                        fatal("%s line %d: too many host keys specified (max %d).",
-                            filename, linenum, MAX_HOSTKEYS);
-                charptr = &options->host_key_files[*intptr];
- parse_filename:
                 arg = strdelim(&cp);
                 if (!arg || *arg == '\0')
                         fatal("%s line %d: missing file name.",
                             filename, linenum);
-                if (*activep && *charptr == NULL) {
-                        *charptr = derelativise_path(arg);
-                        /* increase optional counter */
-                        if (intptr != NULL)
-                                *intptr = *intptr + 1;
-                }
+                if (*activep)
+                        servconf_add_hostkey(filename, linenum, options, arg);
                 break;
 
         case sHostKeyAgent:
@@ -1158,17 +1187,28 @@ process_server_config_line(ServerOptions *options, char *line,
                 break;
 
         case sHostCertificate:
-                intptr = &options->num_host_cert_files;
-                if (*intptr >= MAX_HOSTKEYS)
-                        fatal("%s line %d: too many host certificates "
-                            "specified (max %d).", filename, linenum,
-                            MAX_HOSTCERTS);
-                charptr = &options->host_cert_files[*intptr];
-                goto parse_filename;
+                arg = strdelim(&cp);
+                if (!arg || *arg == '\0')
+                        fatal("%s line %d: missing file name.",
+                            filename, linenum);
+                if (*activep)
+                        servconf_add_hostcert(filename, linenum, options, arg);
+                break;
 
         case sPidFile:
                 charptr = &options->pid_file;
-                goto parse_filename;
+ parse_filename:
+                arg = strdelim(&cp);
+                if (!arg || *arg == '\0')
+                        fatal("%s line %d: missing file name.",
+                            filename, linenum);
+                if (*activep && *charptr == NULL) {
+                        *charptr = derelativise_path(arg);
+                        /* increase optional counter */
+                        if (intptr != NULL)
+                                *intptr = *intptr + 1;
+                }
+                break;
 
         case sPermitRootLogin:
                 intptr = &options->permit_root_login;
@@ -1412,55 +1452,47 @@ process_server_config_line(ServerOptions *options, char *line,
 
         case sAllowUsers:
                 while ((arg = strdelim(&cp)) && *arg != '\0') {
-                        if (options->num_allow_users >= MAX_ALLOW_USERS)
-                                fatal("%s line %d: too many allow users.",
-                                    filename, linenum);
                         if (match_user(NULL, NULL, NULL, arg) == -1)
                                 fatal("%s line %d: invalid AllowUsers pattern: "
                                     "\"%.100s\"", filename, linenum, arg);
                         if (!*activep)
                                 continue;
-                        options->allow_users[options->num_allow_users++] =
-                            xstrdup(arg);
+                        array_append(filename, linenum, "AllowUsers",
+                            &options->allow_users, &options->num_allow_users,
+                            arg);
                 }
                 break;
 
         case sDenyUsers:
                 while ((arg = strdelim(&cp)) && *arg != '\0') {
-                        if (options->num_deny_users >= MAX_DENY_USERS)
-                                fatal("%s line %d: too many deny users.",
-                                    filename, linenum);
                         if (match_user(NULL, NULL, NULL, arg) == -1)
                                 fatal("%s line %d: invalid DenyUsers pattern: "
                                     "\"%.100s\"", filename, linenum, arg);
                         if (!*activep)
                                 continue;
-                        options->deny_users[options->num_deny_users++] =
-                            xstrdup(arg);
+                        array_append(filename, linenum, "DenyUsers",
+                            &options->deny_users, &options->num_deny_users,
+                            arg);
                 }
                 break;
 
         case sAllowGroups:
                 while ((arg = strdelim(&cp)) && *arg != '\0') {
-                        if (options->num_allow_groups >= MAX_ALLOW_GROUPS)
-                                fatal("%s line %d: too many allow groups.",
-                                    filename, linenum);
                         if (!*activep)
                                 continue;
-                        options->allow_groups[options->num_allow_groups++] =
-                            xstrdup(arg);
+                        array_append(filename, linenum, "AllowGroups",
+                            &options->allow_groups, &options->num_allow_groups,
+                            arg);
                 }
                 break;
 
         case sDenyGroups:
                 while ((arg = strdelim(&cp)) && *arg != '\0') {
-                        if (options->num_deny_groups >= MAX_DENY_GROUPS)
-                                fatal("%s line %d: too many deny groups.",
-                                    filename, linenum);
                         if (!*activep)
                                 continue;
-                        options->deny_groups[options->num_deny_groups++] =
-                            xstrdup(arg);
+                        array_append(filename, linenum, "DenyGroups",
+                            &options->deny_groups, &options->num_deny_groups,
+                            arg);
                 }
                 break;
 
@@ -1579,14 +1611,12 @@ process_server_config_line(ServerOptions *options, char *line,
         case sAuthorizedKeysFile:
                 if (*activep && options->num_authkeys_files == 0) {
                         while ((arg = strdelim(&cp)) && *arg != '\0') {
-                                if (options->num_authkeys_files >=
-                                    MAX_AUTHKEYS_FILES)
-                                        fatal("%s line %d: "
-                                            "too many authorized keys files.",
-                                            filename, linenum);
-                                options->authorized_keys_files[
-                                    options->num_authkeys_files++] =
-                                    tilde_expand_filename(arg, getuid());
+                                arg = tilde_expand_filename(arg, getuid());
+                                array_append(filename, linenum,
+                                    "AuthorizedKeysFile",
+                                    &options->authorized_keys_files,
+                                    &options->num_authkeys_files, arg);
+                                free(arg);
                         }
                 }
                 return 0;
@@ -1618,13 +1648,11 @@ process_server_config_line(ServerOptions *options, char *line,
                         if (strchr(arg, '=') != NULL)
                                 fatal("%s line %d: Invalid environment name.",
                                     filename, linenum);
-                        if (options->num_accept_env >= MAX_ACCEPT_ENV)
-                                fatal("%s line %d: too many allow env.",
-                                    filename, linenum);
                         if (!*activep)
                                 continue;
-                        options->accept_env[options->num_accept_env++] =
-                            xstrdup(arg);
+                        array_append(filename, linenum, "AcceptEnv",
+                            &options->accept_env, &options->num_accept_env,
+                            arg);
                 }
                 break;
 
@@ -1684,15 +1712,12 @@ process_server_config_line(ServerOptions *options, char *line,
                                 fatal("%s line %d: bad port number in "
                                     "PermitOpen", filename, linenum);
                         if (*activep && value == 0) {
-                                options->permitted_opens = xrecallocarray(
-                                    options->permitted_opens,
-                                    options->num_permitted_opens,
-                                    options->num_permitted_opens + 1,
-                                    sizeof(*options->permitted_opens));
-                                i = options->num_permitted_opens++;
-                                options->permitted_opens[i] = arg2;
-                        } else
-                                free(arg2);
+                                array_append(filename, linenum,
+                                    "PermitOpen",
+                                    &options->permitted_opens,
+                                    &options->num_permitted_opens, arg2);
+                        }
+                        free(arg2);
                 }
                 break;
 
@@ -1815,11 +1840,6 @@ process_server_config_line(ServerOptions *options, char *line,
                         value = 0; /* seen "any" pseudo-method */
                         value2 = 0; /* sucessfully parsed any method */
                         while ((arg = strdelim(&cp)) && *arg != '\0') {
-                                if (options->num_auth_methods >=
-                                    MAX_AUTH_METHODS)
-                                        fatal("%s line %d: "
-                                            "too many authentication methods.",
-                                            filename, linenum);
                                 if (strcmp(arg, "any") == 0) {
                                         if (options->num_auth_methods > 0) {
                                                 fatal("%s line %d: \"any\" "
@@ -1840,8 +1860,10 @@ process_server_config_line(ServerOptions *options, char *line,
                                 value2 = 1;
                                 if (!*activep)
                                         continue;
-                                options->auth_methods[
-                                    options->num_auth_methods++] = xstrdup(arg);
+                                array_append(filename, linenum,
+                                    "AuthenticationMethods",
+                                    &options->auth_methods,
+                                    &options->num_auth_methods, arg);
                         }
                         if (value2 == 0) {
                                 fatal("%s line %d: no AuthenticationMethods "
@@ -2057,17 +2079,16 @@ copy_set_server_options(ServerOptions *dst, ServerOptions *src, int preauth)
                 dst->n = src->n; \
         } \
 } while(0)
-#define M_CP_STRARRAYOPT(n, num_n) do {\
-        if (src->num_n != 0) { \
-                for (dst->num_n = 0; dst->num_n < src->num_n; dst->num_n++) \
-                        dst->n[dst->num_n] = xstrdup(src->n[dst->num_n]); \
-        } \
-} while(0)
-#define M_CP_STRARRAYOPT_ALLOC(n, num_n) do { \
-        if (src->num_n != 0) { \
-                dst->n = xcalloc(src->num_n, sizeof(*dst->n)); \
-                M_CP_STRARRAYOPT(n, num_n); \
-                dst->num_n = src->num_n; \
+#define M_CP_STRARRAYOPT(s, num_s) do {\
+        u_int i; \
+        if (src->num_s != 0) { \
+                for (i = 0; i < dst->num_s; i++) \
+                        free(dst->s[i]); \
+                free(dst->s); \
+                dst->s = xcalloc(src->num_s, sizeof(*dst->s)); \
+                for (i = 0; i < src->num_s; i++) \
+                        dst->s[i] = xstrdup(src->s[i]); \
+                dst->num_s = src->num_s; \
         } \
 } while(0)
 
@@ -2100,7 +2121,6 @@ copy_set_server_options(ServerOptions *dst, ServerOptions *src, int preauth)
 #undef M_CP_INTOPT
 #undef M_CP_STROPT
 #undef M_CP_STRARRAYOPT
-#undef M_CP_STRARRAYOPT_ALLOC
 
 void
 parse_server_config(ServerOptions *options, const char *filename, Buffer *conf,
diff --git a/sshd.c b/sshd.c
index 51a1aaf6e..2ff385552 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.492 2017/09/12 06:32:07 djm Exp $ */
+/* $OpenBSD: sshd.c,v 1.493 2017/10/05 15:52:03 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -467,7 +467,7 @@ sshd_exchange_identification(struct ssh *ssh, int sock_in, int sock_out)
 void
 destroy_sensitive_data(void)
 {
-        int i;
+        u_int i;
 
         for (i = 0; i < options.num_host_key_files; i++) {
                 if (sensitive_data.host_keys[i]) {
@@ -486,7 +486,7 @@ void
 demote_sensitive_data(void)
 {
         struct sshkey *tmp;
-        int i;
+        u_int i;
 
         for (i = 0; i < options.num_host_key_files; i++) {
                 if (sensitive_data.host_keys[i]) {
@@ -685,7 +685,7 @@ list_hostkey_types(void)
         Buffer b;
         const char *p;
         char *ret;
-        int i;
+        u_int i;
         struct sshkey *key;
 
         buffer_init(&b);
@@ -745,7 +745,7 @@ list_hostkey_types(void)
 static struct sshkey *
 get_hostkey_by_type(int type, int nid, int need_private, struct ssh *ssh)
 {
-        int i;
+        u_int i;
         struct sshkey *key;
 
         for (i = 0; i < options.num_host_key_files; i++) {
@@ -785,7 +785,7 @@ get_hostkey_private_by_type(int type, int nid, struct ssh *ssh)
 struct sshkey *
 get_hostkey_by_index(int ind)
 {
-        if (ind < 0 || ind >= options.num_host_key_files)
+        if (ind < 0 || (u_int)ind >= options.num_host_key_files)
                 return (NULL);
         return (sensitive_data.host_keys[ind]);
 }
@@ -793,7 +793,7 @@ get_hostkey_by_index(int ind)
 struct sshkey *
 get_hostkey_public_by_index(int ind, struct ssh *ssh)
 {
-        if (ind < 0 || ind >= options.num_host_key_files)
+        if (ind < 0 || (u_int)ind >= options.num_host_key_files)
                 return (NULL);
         return (sensitive_data.host_pubkeys[ind]);
 }
@@ -801,7 +801,7 @@ get_hostkey_public_by_index(int ind, struct ssh *ssh)
 int
 get_hostkey_index(struct sshkey *key, int compare, struct ssh *ssh)
 {
-        int i;
+        u_int i;
 
         for (i = 0; i < options.num_host_key_files; i++) {
                 if (key_is_cert(key)) {
@@ -830,7 +830,8 @@ notify_hostkeys(struct ssh *ssh)
 {
         struct sshbuf *buf;
         struct sshkey *key;
-        int i, nkeys, r;
+        u_int i, nkeys;
+        int r;
         char *fp;
 
         /* Some clients cannot cope with the hostkeys message, skip those. */
@@ -861,7 +862,7 @@ notify_hostkeys(struct ssh *ssh)
                 packet_put_string(sshbuf_ptr(buf), sshbuf_len(buf));
                 nkeys++;
         }
-        debug3("%s: sent %d hostkeys", __func__, nkeys);
+        debug3("%s: sent %u hostkeys", __func__, nkeys);
         if (nkeys == 0)
                 fatal("%s: no hostkeys", __func__);
         packet_send();
@@ -1357,13 +1358,12 @@ main(int ac, char **av)
         struct ssh *ssh = NULL;
         extern char *optarg;
         extern int optind;
-        int r, opt, i, j, on = 1, already_daemon;
+        int r, opt, on = 1, already_daemon, remote_port;
         int sock_in = -1, sock_out = -1, newsock = -1;
         const char *remote_ip;
-        int remote_port;
         char *fp, *line, *laddr, *logfile = NULL;
         int config_s[2] = { -1 , -1 };
-        u_int n;
+        u_int i, j;
         u_int64_t ibytes, obytes;
         mode_t new_umask;
         struct sshkey *key;
@@ -1416,12 +1416,8 @@ main(int ac, char **av)
                         config_file_name = optarg;
                         break;
                 case 'c':
-                        if (options.num_host_cert_files >= MAX_HOSTCERTS) {
-                                fprintf(stderr, "too many host certificates.\n");
-                                exit(1);
-                        }
-                        options.host_cert_files[options.num_host_cert_files++] =
-                           derelativise_path(optarg);
+                        servconf_add_hostcert("[command-line]", 0,
+                            &options, optarg);
                         break;
                 case 'd':
                         if (debug_flag == 0) {
@@ -1480,12 +1476,8 @@ main(int ac, char **av)
                         /* protocol 1, ignored */
                         break;
                 case 'h':
-                        if (options.num_host_key_files >= MAX_HOSTKEYS) {
-                                fprintf(stderr, "too many host keys.\n");
-                                exit(1);
-                        }
-                        options.host_key_files[options.num_host_key_files++] = 
-                           derelativise_path(optarg);
+                        servconf_add_hostkey("[command-line]", 0,
+                            &options, optarg);
                         break;
                 case 't':
                         test_flag = 1;
@@ -1611,12 +1603,12 @@ main(int ac, char **av)
          * and warns for trivial misconfigurations that could break login.
          */
         if (options.num_auth_methods != 0) {
-                for (n = 0; n < options.num_auth_methods; n++) {
-                        if (auth2_methods_valid(options.auth_methods[n],
+                for (i = 0; i < options.num_auth_methods; i++) {
+                        if (auth2_methods_valid(options.auth_methods[i],
                             1) == 0)
                                 break;
                 }
-                if (n >= options.num_auth_methods)
+                if (i >= options.num_auth_methods)
                         fatal("AuthenticationMethods cannot be satisfied by "
                             "enabled authentication methods");
         }
@@ -1752,7 +1744,7 @@ main(int ac, char **av)
                         continue;
                 }
                 sensitive_data.host_certificates[j] = key;
-                debug("host certificate: #%d type %d %s", j, key->type,
+                debug("host certificate: #%u type %d %s", j, key->type,
                     key_type(key));
         }
 
@@ -1796,8 +1788,10 @@ main(int ac, char **av)
                 debug("setgroups() failed: %.200s", strerror(errno));
 
         if (rexec_flag) {
+                if (rexec_argc < 0)
+                        fatal("rexec_argc %d < 0", rexec_argc);
                 rexec_argv = xcalloc(rexec_argc + 2, sizeof(char *));
-                for (i = 0; i < rexec_argc; i++) {
+                for (i = 0; i < (u_int)rexec_argc; i++) {
                         debug("rexec_argv[%d]='%s'", i, saved_argv[i]);
                         rexec_argv[i] = saved_argv[i];
                 }