diff options
| -rw-r--r-- | modules/pam_env/pam_env.c | 156 | ||||
| -rw-r--r-- | modules/pam_env/tst-pam_env-retval.c | 7 |
2 files changed, 123 insertions, 40 deletions
diff --git a/modules/pam_env/pam_env.c b/modules/pam_env/pam_env.c index a214593c..592bc4b8 100644 --- a/modules/pam_env/pam_env.c +++ b/modules/pam_env/pam_env.c @@ -21,6 +21,7 @@ #include <errno.h> #include <pwd.h> #include <stdarg.h> +#include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -62,6 +63,12 @@ typedef struct var { #define UNDEFINE_VAR 102 #define ILLEGAL_VAR 103 +struct string_buffer { + char *str; + size_t len; + size_t size; +}; + /* This is a special value used to designate an empty string */ static char quote='\0'; @@ -568,14 +575,105 @@ _pam_get_item_byname(pam_handle_t *pamh, const char *name) return itemval; } +static void +_strbuf_init(struct string_buffer *buffer) +{ + buffer->str = NULL; + buffer->len = 0; + buffer->size = 0; +} + +static void +_strbuf_free(struct string_buffer *buffer) +{ + pam_overwrite_n(buffer->str, buffer->len); + _pam_drop(buffer->str); + buffer->len = 0; + buffer->size = 0; +} + +/* + * Allocates given amount of bytes in buffer for addition. + * Internally adding an extra byte for NUL character. + * + * Returns 0 on success, 1 on error. + */ +static int +_strbuf_reserve(struct string_buffer *buffer, size_t add) +{ + char *p; + size_t s; + + /* Is already enough memory allocated? */ + if (add < buffer->size - buffer->len) { + return 0; + } + + /* Can the requested bytes (plus additional NUL byte) fit at all? */ + if (buffer->len >= SIZE_MAX - add) { + return 1; + } + + if (buffer->size == 0 && add < 64) { + /* Start with 64 bytes if that's enough */ + s = 64; + } else if (buffer->size >= SIZE_MAX / 2 || buffer->size * 2 < add + 1) { + /* If doubling is not enough (or not possible), get as much as needed */ + s = buffer->len + add + 1; + } else { + /* Ideally, double allocated memory */ + s = buffer->size * 2; + } + + if ((p = realloc(buffer->str, s)) == NULL) { + return 1; + } + + buffer->str = p; + buffer->size = s; + + return 0; +} + +static int +_strbuf_add_char(struct string_buffer *buffer, char c) +{ + D(("Called <%s> + <%c>.", buffer->str == NULL ? "" : buffer->str, c)); + + if (_strbuf_reserve(buffer, 1)) { + return 1; + } + + buffer->str[buffer->len++] = c; + buffer->str[buffer->len] = '\0'; + + return 0; +} + +static int +_strbuf_add_string(struct string_buffer *buffer, const char *str) +{ + size_t len = strlen(str); + + D(("Called <%s> + <%s>.", buffer->str == NULL ? "" : buffer->str, str)); + + if (_strbuf_reserve(buffer, len)) { + return 1; + } + + strcpy(buffer->str + buffer->len, str); + buffer->len += len; + + return 0; +} + static int _expand_arg(pam_handle_t *pamh, char **value) { const char *orig=*value; + struct string_buffer buf; - /* I know this shouldn't be hard-coded but it's so much easier this way */ - char tmp[MAX_ENV] = {}; - size_t idx = 0; + _strbuf_init(&buf); /* * (possibly non-existent) environment variables can be used as values @@ -593,14 +691,11 @@ _expand_arg(pam_handle_t *pamh, char **value) pam_syslog(pamh, LOG_ERR, "Unrecognized escaped character: <%c> - ignoring", *orig); - } else if (idx + 1 < MAX_ENV) { - tmp[idx++] = *orig++; /* Note the increment */ } else { - /* is it really a good idea to try to log this? */ - D(("Variable buffer overflow: <%s> + <%c>", tmp, *orig)); - pam_syslog (pamh, LOG_ERR, "Variable buffer overflow: <%s> + <%c>", - tmp, *orig); - goto buf_err; + /* Note the increment */ + if (_strbuf_add_char(&buf, *orig++)) { + goto buf_err; + } } continue; } @@ -610,8 +705,9 @@ _expand_arg(pam_handle_t *pamh, char **value) " <%s> - ignoring", orig)); pam_syslog(pamh, LOG_ERR, "Expandable variables must be wrapped in {}" " <%s> - ignoring", orig); - if (idx + 1 < MAX_ENV) { - tmp[idx++] = *orig++; /* Note the increment */ + /* Note the increment */ + if (_strbuf_add_char(&buf, *orig++)) { + goto buf_err; } continue; } else { @@ -662,51 +758,37 @@ _expand_arg(pam_handle_t *pamh, char **value) } /* switch */ if (tmpptr) { - size_t len = strlen(tmpptr); - if (idx + len < MAX_ENV) { - strcpy(tmp + idx, tmpptr); - idx += len; - } else { - /* is it really a good idea to try to log this? */ - D(("Variable buffer overflow: <%s> + <%s>", tmp, tmpptr)); - pam_syslog (pamh, LOG_ERR, - "Variable buffer overflow: <%s> + <%s>", tmp, tmpptr); + if (_strbuf_add_string(&buf, tmpptr)) { goto buf_err; } } } /* if ('{' != *orig++) */ } else { /* if ( '$' == *orig || '@' == *orig) */ - if (idx + 1 < MAX_ENV) { - tmp[idx++] = *orig++; /* Note the increment */ - } else { - /* is it really a good idea to try to log this? */ - D(("Variable buffer overflow: <%s> + <%c>", tmp, *orig)); - pam_syslog(pamh, LOG_ERR, - "Variable buffer overflow: <%s> + <%c>", tmp, *orig); + /* Note the increment */ + if (_strbuf_add_char(&buf, *orig++)) { goto buf_err; } } } /* for (;*orig;) */ - if (idx > strlen(*value)) { + if (buf.len > strlen(*value)) { free(*value); - if ((*value = malloc(idx + 1)) == NULL) { - D(("Couldn't malloc %zu bytes for expanded var", idx + 1)); - pam_syslog (pamh, LOG_CRIT, "Couldn't malloc %zu bytes for expanded var", - idx+1); + if ((*value = strdup(buf.str)) == NULL) { goto buf_err; } + } else { + const char *tmpptr = buf.str == NULL ? "" : buf.str; + strcpy(*value, tmpptr); } - strcpy(*value, tmp); - pam_overwrite_array(tmp); + _strbuf_free(&buf); D(("Exit.")); return PAM_SUCCESS; buf_err: - pam_overwrite_array(tmp); + _strbuf_free(&buf); return PAM_BUF_ERR; abort_err: - pam_overwrite_array(tmp); + _strbuf_free(&buf); return PAM_ABORT; } diff --git a/modules/pam_env/tst-pam_env-retval.c b/modules/pam_env/tst-pam_env-retval.c index 6e5558f2..68f6f901 100644 --- a/modules/pam_env/tst-pam_env-retval.c +++ b/modules/pam_env/tst-pam_env-retval.c @@ -70,7 +70,8 @@ setup(void) ASSERT_NE(NULL, fp = fopen(my_conf, "w")); ASSERT_LT(0, fprintf(fp, "EDITOR\tDEFAULT=vi DEFAULT= DEFAULT=vim\n" - "PAGER\tDEFAULT=more\n")); + "PAGER\tDEFAULT=more\n" + "NAME\tDEFAULT=@{PAM_USER}\n")); ASSERT_EQ(0, fclose(fp)); ASSERT_NE(NULL, fp = fopen(my_env, "w")); @@ -127,7 +128,7 @@ check_env(const char **list) pam_handle_t *pamh = NULL; ASSERT_EQ(PAM_SUCCESS, - pam_start_confdir(service_file, "", &conv, ".", &pamh)); + pam_start_confdir(service_file, "user", &conv, ".", &pamh)); ASSERT_NE(NULL, pamh); ASSERT_EQ(PAM_SUCCESS, pam_open_session(pamh, 0)); @@ -231,7 +232,7 @@ main(void) cwd, my_conf, "/dev/null")); ASSERT_EQ(0, fclose(fp)); - const char *env1[] = { "EDITOR=vim", "PAGER=more", NULL }; + const char *env1[] = { "EDITOR=vim", "PAGER=more", "NAME=user", NULL }; check_env(env1); /* |