From 010359b32659f455fddd2bd85fd7cc4d7a3b994a Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Sun, 6 Nov 2016 05:46:37 +0000 Subject: [PATCH] upstream commit Validate address ranges for AllowUser/DenyUsers at configuration load time and refuse to accept bad ones. It was previously possible to specify invalid CIDR address ranges (e.g. djm@127.1.2.3/55) and these would always match. Thanks to Laurence Parry for a detailed bug report. ok markus (for a previous diff version) Upstream-ID: 9dfcdd9672b06e65233ea4434c38226680d40bfb --- auth.c | 22 ++++++++++++++++------ match.c | 21 +++++++++++++++------ servconf.c | 8 +++++++- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/auth.c b/auth.c index b6a440213..f7c1e7f3b 100644 --- a/auth.c +++ b/auth.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth.c,v 1.116 2016/08/13 17:47:41 markus Exp $ */ +/* $OpenBSD: auth.c,v 1.117 2016/11/06 05:46:37 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -103,6 +103,7 @@ allowed_user(struct passwd * pw) struct stat st; const char *hostname = NULL, *ipaddr = NULL, *passwd = NULL; u_int i; + int r; #ifdef USE_SHADOW struct spwd *spw = NULL; #endif @@ -192,8 +193,12 @@ allowed_user(struct passwd * pw) /* Return false if user is listed in DenyUsers */ if (options.num_deny_users > 0) { for (i = 0; i < options.num_deny_users; i++) - if (match_user(pw->pw_name, hostname, ipaddr, - options.deny_users[i])) { + r = match_user(pw->pw_name, hostname, ipaddr, + options.deny_users[i]); + if (r < 0) { + fatal("Invalid DenyUsers pattern \"%.100s\"", + options.deny_users[i]); + } else if (r != 1) { logit("User %.100s from %.100s not allowed " "because listed in DenyUsers", pw->pw_name, hostname); @@ -202,10 +207,15 @@ allowed_user(struct passwd * pw) } /* Return false if AllowUsers isn't empty and user isn't listed there */ if (options.num_allow_users > 0) { - for (i = 0; i < options.num_allow_users; i++) - if (match_user(pw->pw_name, hostname, ipaddr, - options.allow_users[i])) + for (i = 0; i < options.num_allow_users; i++) { + r = match_user(pw->pw_name, hostname, ipaddr, + options.allow_users[i]); + if (r < 0) { + fatal("Invalid AllowUsers pattern \"%.100s\"", + options.allow_users[i]); + } else if (r == 1) break; + } /* i < options.num_allow_users iff we break for loop */ if (i >= options.num_allow_users) { logit("User %.100s from %.100s not allowed because " diff --git a/match.c b/match.c index b29a30e91..c15dcd1ef 100644 --- a/match.c +++ b/match.c @@ -1,4 +1,4 @@ -/* $OpenBSD: match.c,v 1.32 2016/09/21 16:55:42 djm Exp $ */ +/* $OpenBSD: match.c,v 1.33 2016/11/06 05:46:37 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -191,11 +191,10 @@ match_host_and_ip(const char *host, const char *ipaddr, { int mhost, mip; - /* error in ipaddr match */ if ((mip = addr_match_list(ipaddr, patterns)) == -2) - return -1; - else if (mip == -1) /* negative ip address match */ - return 0; + return -1; /* error in ipaddr match */ + else if (host == NULL || ipaddr == NULL || mip == -1) + return 0; /* negative ip address match, or testing pattern */ /* negative hostname match */ if ((mhost = match_hostname(host, patterns)) == -1) @@ -207,7 +206,9 @@ match_host_and_ip(const char *host, const char *ipaddr, } /* - * match user, user@host_or_ip, user@host_or_ip_list against pattern + * Match user, user@host_or_ip, user@host_or_ip_list against pattern. + * If user, host and ipaddr are all NULL then validate pattern/ + * Returns -1 on invalid pattern, 0 on no match, 1 on match. */ int match_user(const char *user, const char *host, const char *ipaddr, @@ -216,6 +217,14 @@ match_user(const char *user, const char *host, const char *ipaddr, char *p, *pat; int ret; + /* test mode */ + if (user == NULL && host == NULL && ipaddr == NULL) { + if ((p = strchr(pattern, '@')) != NULL && + match_host_and_ip(NULL, NULL, p + 1) < 0) + return -1; + return 0; + } + if ((p = strchr(pattern,'@')) == NULL) return match_pattern(user, pattern); diff --git a/servconf.c b/servconf.c index 35abec489..a18ebb597 100644 --- a/servconf.c +++ b/servconf.c @@ -1,5 +1,5 @@ -/* $OpenBSD: servconf.c,v 1.298 2016/10/24 01:09:17 dtucker Exp $ */ +/* $OpenBSD: servconf.c,v 1.299 2016/11/06 05:46:37 djm Exp $ */ /* * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland * All rights reserved @@ -1366,6 +1366,9 @@ process_server_config_line(ServerOptions *options, char *line, 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++] = @@ -1378,6 +1381,9 @@ process_server_config_line(ServerOptions *options, char *line, 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++] =