From eae003b59c6a4773f2f3c7d67381ace01a1ddca6 Mon Sep 17 00:00:00 2001 From: Theo de Raadt Date: Wed, 4 Dec 2019 06:25:46 +0000 Subject: libc's authentication privsep layer performed insufficient username validation. Repair work mostly by markus and millert, first of all solving the primary problem, then adding some additional validation points. And then futher validation in login and su. This will be 6.5/021_libcauth.patch.sig and 6.6/010_libcauth.patch.sig Reported by Qualys --- lib/libc/gen/auth_subr.c | 12 +++++++++--- lib/libc/gen/authenticate.c | 37 ++++++++++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/libc/gen/auth_subr.c b/lib/libc/gen/auth_subr.c index 6d1844f4a28..1286a96fe40 100644 --- a/lib/libc/gen/auth_subr.c +++ b/lib/libc/gen/auth_subr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth_subr.c,v 1.53 2019/06/28 13:32:41 deraadt Exp $ */ +/* $OpenBSD: auth_subr.c,v 1.54 2019/12/04 06:25:45 deraadt Exp $ */ /* * Copyright (c) 2000-2002,2004 Todd C. Miller @@ -304,7 +304,8 @@ auth_challenge(auth_session_t *as) char path[PATH_MAX]; int len; - if (as == NULL || as->style == NULL || as->name == NULL) + if (as == NULL || as->style == NULL || as->name == NULL || + !_auth_validuser(as->name)) return (NULL); len = snprintf(path, sizeof(path), _PATH_AUTHPROG "%s", as->style); @@ -316,7 +317,7 @@ auth_challenge(auth_session_t *as) free(as->challenge); as->challenge = NULL; - auth_call(as, path, as->style, "-s", "challenge", as->name, + auth_call(as, path, as->style, "-s", "challenge", "--", as->name, as->class, (char *)NULL); if (as->state & AUTH_CHALLENGE) as->challenge = auth_getvalue(as, "challenge"); @@ -476,6 +477,10 @@ auth_setitem(auth_session_t *as, auth_item_t item, char *value) case AUTHV_NAME: if (value == as->name) return (0); + if (value != NULL && !_auth_validuser(value)) { + errno = EINVAL; + return (-1); + } if (value != NULL && (value = strdup(value)) == NULL) return (-1); free(as->name); @@ -821,6 +826,7 @@ auth_call(auth_session_t *as, char *path, ...) argv[argc++] = "-v"; argv[argc++] = "fd=4"; /* AUTH_FD, see below */ } + /* XXX - fail if out of space in argv */ for (opt = as->optlist; opt != NULL; opt = opt->next) { if (argc < Nargc - 2) { argv[argc++] = "-v"; diff --git a/lib/libc/gen/authenticate.c b/lib/libc/gen/authenticate.c index fb96881832f..5fb6853888a 100644 --- a/lib/libc/gen/authenticate.c +++ b/lib/libc/gen/authenticate.c @@ -1,4 +1,4 @@ -/* $OpenBSD: authenticate.c,v 1.27 2019/06/28 13:32:41 deraadt Exp $ */ +/* $OpenBSD: authenticate.c,v 1.28 2019/12/04 06:25:45 deraadt Exp $ */ /*- * Copyright (c) 1997 Berkeley Software Design, Inc. All rights reserved. @@ -173,6 +173,17 @@ auth_cat(char *file) } DEF_WEAK(auth_cat); +int +_auth_validuser(const char *name) +{ + /* User name must be specified and may not start with a '-'. */ + if (*name == '\0' || *name == '-') { + syslog(LOG_ERR, "invalid user name %s", name); + return 0; + } + return 1; +} + int auth_approval(auth_session_t *as, login_cap_t *lc, char *name, char *type) { @@ -192,6 +203,10 @@ auth_approval(auth_session_t *as, login_cap_t *lc, char *name, char *type) if (pwd == NULL) { if (name != NULL) { + if (!_auth_validuser(name)) { + warnx("cannot approve who we don't recognize"); + return (0); + } getpwnam_r(name, &pwstore, pwbuf, sizeof(pwbuf), &pwd); } else { getpwuid_r(getuid(), &pwstore, pwbuf, sizeof(pwbuf), @@ -217,7 +232,7 @@ auth_approval(auth_session_t *as, login_cap_t *lc, char *name, char *type) } if (pwd == NULL && (approve = strchr(name, '.')) != NULL) { strlcpy(path, name, sizeof path); - path[approve-name] = '\0'; + path[approve - name] = '\0'; getpwnam_r(name, &pwstore, pwbuf, sizeof(pwbuf), &pwd); } lc = login_getclass(pwd ? pwd->pw_class : NULL); @@ -290,7 +305,7 @@ auth_approval(auth_session_t *as, login_cap_t *lc, char *name, char *type) } } if (approve) - auth_call(as, approve, strrchr(approve, '/') + 1, name, + auth_call(as, approve, strrchr(approve, '/') + 1, "--", name, lc->lc_class, type, (char *)NULL); out: @@ -314,6 +329,8 @@ auth_usercheck(char *name, char *style, char *type, char *password) struct passwd pwstore, *pwd = NULL; char *slash; + if (!_auth_validuser(name)) + return (NULL); if (strlcpy(namebuf, name, sizeof(namebuf)) >= sizeof(namebuf)) return (NULL); name = namebuf; @@ -382,6 +399,8 @@ auth_userchallenge(char *name, char *style, char *type, char **challengep) struct passwd pwstore, *pwd = NULL; char *slash, pwbuf[_PW_BUF_LEN]; + if (!_auth_validuser(name)) + return (NULL); if (strlen(name) >= sizeof(namebuf)) return (NULL); strlcpy(namebuf, name, sizeof namebuf); @@ -440,7 +459,8 @@ auth_userresponse(auth_session_t *as, char *response, int more) auth_setstate(as, 0); if ((style = auth_getitem(as, AUTHV_STYLE)) == NULL || - (name = auth_getitem(as, AUTHV_NAME)) == NULL) { + (name = auth_getitem(as, AUTHV_NAME)) == NULL || + !_auth_validuser(name)) { if (more == 0) return (auth_close(as)); return(0); @@ -466,7 +486,8 @@ auth_userresponse(auth_session_t *as, char *response, int more) } else auth_setdata(as, "", 1); - auth_call(as, path, style, "-s", "response", name, class, (char *)NULL); + auth_call(as, path, style, "-s", "response", "--", name, + class, (char *)NULL); /* * If they authenticated then make sure they did not expire @@ -495,7 +516,7 @@ auth_verify(auth_session_t *as, char *style, char *name, ...) char path[PATH_MAX]; if ((name == NULL || style == NULL) && as == NULL) - return (as); + return (NULL); if (as == NULL && (as = auth_open()) == NULL) return (NULL); @@ -509,12 +530,14 @@ auth_verify(auth_session_t *as, char *style, char *name, ...) style = auth_getitem(as, AUTHV_STYLE); name = auth_getitem(as, AUTHV_NAME); + if (!_auth_validuser(name)) + return (as); snprintf(path, sizeof(path), _PATH_AUTHPROG "%s", style); va_start(ap, name); auth_set_va_list(as, ap); auth_call(as, path, auth_getitem(as, AUTHV_STYLE), "-s", - auth_getitem(as, AUTHV_SERVICE), name, (char *)NULL); + auth_getitem(as, AUTHV_SERVICE), "--", name, (char *)NULL); va_end(ap); return (as); } -- cgit v1.2.3