make authorized_keys environment="..." directives first-match-wins

and more strictly limit their maximum number; prompted by OOM
reported by OSS-fuzz (35470).

feedback and ok dtucker@

diff --git a/usr.bin/ssh/auth-options.c b/usr.bin/ssh/auth-options.c
index a2652c4..7544573 100644 (file)
--- a/usr.bin/ssh/auth-options.c
+++ b/usr.bin/ssh/auth-options.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth-options.c,v 1.95 2021/04/03 06:18:40 djm Exp $ */
+/* $OpenBSD: auth-options.c,v 1.96 2021/07/23 03:57:20 djm Exp $ */
 /*
  * Copyright (c) 2018 Damien Miller <djm@mindrot.org>
  *
@@ -321,6 +321,7 @@ sshauthopt_parse(const char *opts, const char **errstrp)
        struct sshauthopt *ret = NULL;
        const char *errstr = "unknown error";
        uint64_t valid_before;
+       size_t i, l;
 
        if (errstrp != NULL)
                *errstrp = NULL;
@@ -394,7 +395,7 @@ sshauthopt_parse(const char *opts, const char **errstrp)
                            valid_before < ret->valid_before)
                                ret->valid_before = valid_before;
                } else if (opt_match(&opts, "environment")) {
-                       if (ret->nenv > INT_MAX) {
+                       if (ret->nenv > SSH_AUTHOPT_ENV_MAX) {
                                errstr = "too many environment strings";
                                goto fail;
                        }
@@ -408,23 +409,35 @@ sshauthopt_parse(const char *opts, const char **errstrp)
                        }
                        if ((cp = strdup(opt)) == NULL)
                                goto alloc_fail;
-                       cp[tmp - opt] = '\0'; /* truncate at '=' */
+                       l = (size_t)(tmp - opt);
+                       cp[l] = '\0'; /* truncate at '=' */
                        if (!valid_env_name(cp)) {
                                free(cp);
                                free(opt);
                                errstr = "invalid environment string";
                                goto fail;
                        }
+                       /* Check for duplicates; XXX O(n*log(n)) */
+                       for (i = 0; i < ret->nenv; i++) {
+                               if (strncmp(ret->env[i], cp, l) == 0 &&
+                                   ret->env[i][l] == '=')
+                                       break;
+                       }
                        free(cp);
-                       /* Append it. */
-                       oarray = ret->env;
-                       if ((ret->env = recallocarray(ret->env, ret->nenv,
-                           ret->nenv + 1, sizeof(*ret->env))) == NULL) {
-                               free(opt);
-                               ret->env = oarray; /* put it back for cleanup */
-                               goto alloc_fail;
+                       /* First match wins */
+                       if (i >= ret->nenv) {
+                               /* Append it. */
+                               oarray = ret->env;
+                               if ((ret->env = recallocarray(ret->env,
+                                   ret->nenv, ret->nenv + 1,
+                                   sizeof(*ret->env))) == NULL) {
+                                       free(opt);
+                                       /* put it back for cleanup */
+                                       ret->env = oarray;
+                                       goto alloc_fail;
+                               }
+                               ret->env[ret->nenv++] = opt;
                        }
-                       ret->env[ret->nenv++] = opt;
                } else if (opt_match(&opts, "permitopen")) {
                        if (handle_permit(&opts, 0, &ret->permitopen,
                            &ret->npermitopen, &errstr) != 0)

diff --git a/usr.bin/ssh/auth-options.h b/usr.bin/ssh/auth-options.h
index 118a320..6e29b72 100644 (file)
--- a/usr.bin/ssh/auth-options.h
+++ b/usr.bin/ssh/auth-options.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth-options.h,v 1.30 2020/08/27 01:07:09 djm Exp $ */
+/* $OpenBSD: auth-options.h,v 1.31 2021/07/23 03:57:20 djm Exp $ */
 
 /*
  * Copyright (c) 2018 Damien Miller <djm@mindrot.org>
@@ -23,7 +23,10 @@ struct passwd;
 struct sshkey;
 
 /* Maximum number of permitopen/permitlisten directives to accept */
-#define SSH_AUTHOPT_PERMIT_MAX 4096
+#define SSH_AUTHOPT_PERMIT_MAX 4096
+
+/* Maximum number of environment directives to accept */
+#define SSH_AUTHOPT_ENV_MAX    1024
 
 /*
  * sshauthopt represents key options parsed from authorized_keys or