From 7dbd5c38b794fbd0e8a7a6a053c5daaec9802c0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dag-Erling=20Sm=C3=B8rgrav?= Date: Tue, 3 Jun 2014 21:27:48 +0000 Subject: [PATCH] In openpam_parse_chain(): 1. Finish a comment which was meant to describe the four different termination conditions for the loop in openpam_parse_chain() but ended in mid-sentence. 2. Ensure that errno is consistently set to EINVAL if a syntax error is encountered in the policy file. 3. If openpam_load_module() fails because the module could not be loaded, set errno to ENOEXEC instead of ENOENT. This closes a hole where a missing module or a typo in a module name would cause the corresponding chain to fail open. Normally, if the policy exists but cannot be loaded, openpam_load_chain() will return an error, and openpam_configure() will discard any partially constructed chains. However, openpam_load_chain() interprets ENOENT to mean that the policy was not found, so it does not immediately return an error, the partially-loaded chain is not discarded, and the policy is incorrectly considered to have been successfully loaded. 4. Ensure that errors encountered while parsing an included policy are correctly propagated to the original policy, and that ENOENT while processing an include directive is a hard error, not a soft error. CVE-2014-3879 git-svn-id: svn+ssh://svn.openpam.org/svn/openpam/trunk@795 185d5e19-27fe-0310-9dcf-9bff6b9f3609 --- HISTORY | 4 ++++ lib/libpam/openpam_configure.c | 40 ++++++++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/HISTORY b/HISTORY index 890adac..9285971 100644 --- a/HISTORY +++ b/HISTORY @@ -1,5 +1,9 @@ OpenPAM ?????????? 2014-??-?? + - BUGFIX: Under certain circumstances, specifying a non-existent + module (or misspelling the name of a module) in a policy could + result in a fail-open scenario. + - FEATURE: Add a pam_oath module that implements RFC 4226 (HOTP) and RFC 6238 (TOTP). diff --git a/lib/libpam/openpam_configure.c b/lib/libpam/openpam_configure.c index be1d3b6..96264de 100644 --- a/lib/libpam/openpam_configure.c +++ b/lib/libpam/openpam_configure.c @@ -1,6 +1,6 @@ /*- * Copyright (c) 2001-2003 Networks Associates Technology, Inc. - * Copyright (c) 2004-2012 Dag-Erling Smørgrav + * Copyright (c) 2004-2014 Dag-Erling Smørgrav * All rights reserved. * * This software was developed for the FreeBSD Project by ThinkSec AS and @@ -193,6 +193,7 @@ openpam_parse_chain(pam_handle_t *pamh, openpam_log(PAM_LOG_ERROR, "%s(%d): missing or invalid facility", filename, lineno); + errno = EINVAL; goto fail; } if (facility != fclt && facility != PAM_FACILITY_ANY) { @@ -208,18 +209,28 @@ openpam_parse_chain(pam_handle_t *pamh, openpam_log(PAM_LOG_ERROR, "%s(%d): missing or invalid service name", filename, lineno); + errno = EINVAL; goto fail; } if (wordv[i] != NULL) { openpam_log(PAM_LOG_ERROR, "%s(%d): garbage at end of line", filename, lineno); + errno = EINVAL; goto fail; } ret = openpam_load_chain(pamh, servicename, fclt); FREEV(wordc, wordv); - if (ret < 0) + if (ret < 0) { + /* + * Bogus errno, but this ensures that the + * outer loop does not just ignore the + * error and keep searching. + */ + if (errno == ENOENT) + errno = EINVAL; goto fail; + } continue; } @@ -229,6 +240,7 @@ openpam_parse_chain(pam_handle_t *pamh, openpam_log(PAM_LOG_ERROR, "%s(%d): missing or invalid control flag", filename, lineno); + errno = EINVAL; goto fail; } @@ -238,6 +250,7 @@ openpam_parse_chain(pam_handle_t *pamh, openpam_log(PAM_LOG_ERROR, "%s(%d): missing or invalid module name", filename, lineno); + errno = EINVAL; goto fail; } @@ -247,8 +260,11 @@ openpam_parse_chain(pam_handle_t *pamh, this->flag = ctlf; /* load module */ - if ((this->module = openpam_load_module(modulename)) == NULL) + if ((this->module = openpam_load_module(modulename)) == NULL) { + if (errno == ENOENT) + errno = ENOEXEC; goto fail; + } /* * The remaining items in wordv are the module's @@ -281,7 +297,11 @@ openpam_parse_chain(pam_handle_t *pamh, * The loop ended because openpam_readword() returned NULL, which * can happen for four different reasons: an I/O error (ferror(f) * is true), a memory allocation failure (ferror(f) is false, - * errno is non-zero) + * feof(f) is false, errno is non-zero), the file ended with an + * unterminated quote or backslash escape (ferror(f) is false, + * feof(f) is true, errno is non-zero), or the end of the file was + * reached without error (ferror(f) is false, feof(f) is true, + * errno is zero). */ if (ferror(f) || errno != 0) goto syserr; @@ -406,6 +426,9 @@ openpam_load_chain(pam_handle_t *pamh, } ret = openpam_load_file(pamh, service, facility, filename, style); + /* success */ + if (ret > 0) + RETURNN(ret); /* the file exists, but an error occurred */ if (ret == -1 && errno != ENOENT) RETURNN(ret); @@ -415,7 +438,8 @@ openpam_load_chain(pam_handle_t *pamh, } /* no hit */ - RETURNN(0); + errno = ENOENT; + RETURNN(-1); } /* @@ -436,8 +460,10 @@ openpam_configure(pam_handle_t *pamh, openpam_log(PAM_LOG_ERROR, "invalid service name"); RETURNC(PAM_SYSTEM_ERR); } - if (openpam_load_chain(pamh, service, PAM_FACILITY_ANY) < 0) - goto load_err; + if (openpam_load_chain(pamh, service, PAM_FACILITY_ANY) < 0) { + if (errno != ENOENT) + goto load_err; + } for (fclt = 0; fclt < PAM_NUM_FACILITIES; ++fclt) { if (pamh->chains[fclt] != NULL) continue;