autofs-5.0.4 - make MAX_ERR_BUF and PARSE_MAX_BUF use easier to audit From: Valerie Aurora Henson Non-critical changes to make auditing buffer lengths easier. * Some buffers were the wrong (too big) size, some were used twice for different purposes. * Use sizeof(buf) instead of repeating the *MAX* define in functions that need to know the size of a statically allocated buffer. * Fix a compiler warning about discarding the const on a string. --- CHANGELOG | 1 + modules/lookup_ldap.c | 51 ++++++++++++++++++++++--------------------------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index afd1335..417a001 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ - add "forcestart" and "forcerestart" init script options to allow use of 5.0.3 strartup behavior if required. - always read entire file map into cache to speed lookups. +- make MAX_ERR_BUF and PARSE_MAX_BUF use easier to audit. 4/11/2008 autofs-5.0.4 ----------------------- diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c index bee97ae..d8a60d3 100644 --- a/modules/lookup_ldap.c +++ b/modules/lookup_ldap.c @@ -272,7 +272,7 @@ LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_conte static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt, const char *class, const char *key) { - char buf[PARSE_MAX_BUF]; + char buf[MAX_ERR_BUF]; char *query, *dn, *qdn; LDAPMessage *result, *e; struct ldap_searchdn *sdns = NULL; @@ -296,7 +296,7 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt query = alloca(l); if (query == NULL) { - char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + char *estr = strerror_r(errno, buf, sizeof(buf)); crit(logopt, MODPREFIX "alloca: %s", estr); return NSS_STATUS_UNAVAIL; } @@ -1082,7 +1082,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c } if (!tmp) { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); return 0; } @@ -1104,7 +1104,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c tmp = malloc(l + 1); if (!tmp) { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); crit(logopt, MODPREFIX "malloc: %s", estr); return 0; } @@ -1139,7 +1139,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c /* Isolate the server's name. */ if (!tmp) { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); return 0; } @@ -1180,7 +1180,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c ctxt->mapname = map; else { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); if (ctxt->server) free(ctxt->server); @@ -1191,7 +1191,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c base = malloc(l + 1); if (!base) { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); if (ctxt->server) free(ctxt->server); @@ -1205,7 +1205,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c char *map = malloc(l + 1); if (!map) { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); if (ctxt->server) free(ctxt->server); @@ -1318,7 +1318,7 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co /* If we can't build a context, bail. */ ctxt = malloc(sizeof(struct lookup_context)); if (!ctxt) { - char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + char *estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); return 1; } @@ -1419,8 +1419,9 @@ int lookup_read_master(struct master *master, time_t age, void *context) unsigned int timeout = master->default_timeout; unsigned int logging = master->default_logging; unsigned int logopt = master->logopt; - int rv, l, count, blen; - char buf[PARSE_MAX_BUF]; + int rv, l, count; + char buf[MAX_ERR_BUF]; + char parse_buf[PARSE_MAX_BUF]; char *query; LDAPMessage *result, *e; char *class, *info, *entry; @@ -1442,7 +1443,7 @@ int lookup_read_master(struct master *master, time_t age, void *context) query = alloca(l); if (query == NULL) { - char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + char *estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "alloca: %s", estr); return NSS_STATUS_UNAVAIL; } @@ -1532,19 +1533,13 @@ int lookup_read_master(struct master *master, time_t age, void *context) goto next; } - blen = strlen(*keyValue) + 1 + strlen(*values) + 2; - if (blen > PARSE_MAX_BUF) { + if (snprintf(parse_buf, sizeof(parse_buf), "%s %s", + *keyValue, *values) >= sizeof(parse_buf)) { error(logopt, MODPREFIX "map entry too long"); ldap_value_free(values); goto next; } - memset(buf, 0, PARSE_MAX_BUF); - - strcpy(buf, *keyValue); - strcat(buf, " "); - strcat(buf, *values); - - master_parse_entry(buf, timeout, logging, age); + master_parse_entry(parse_buf, timeout, logging, age); next: ldap_value_free(keyValue); e = ldap_next_entry(ldap, e); @@ -1561,7 +1556,7 @@ static int get_percent_decoded_len(const char *name) { int escapes = 0; int escaped = 0; - char *tmp = name; + const char *tmp = name; int look_for_close = 0; while (*tmp) { @@ -2060,7 +2055,7 @@ static int do_get_entries(struct ldap_search_params *sp, struct map_source *sour mapent = malloc(v_len + 1); if (!mapent) { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); ldap_value_free_len(bvValues); goto next; @@ -2080,7 +2075,7 @@ static int do_get_entries(struct ldap_search_params *sp, struct map_source *sour mapent_len = new_size; } else { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "realloc: %s", estr); } } @@ -2181,7 +2176,7 @@ static int read_one_map(struct autofs_point *ap, sp.query = alloca(l); if (sp.query == NULL) { - char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + char *estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); return NSS_STATUS_UNAVAIL; } @@ -2335,7 +2330,7 @@ static int lookup_one(struct autofs_point *ap, query = alloca(l); if (query == NULL) { - char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + char *estr = strerror_r(errno, buf, sizeof(buf)); crit(ap->logopt, MODPREFIX "malloc: %s", estr); if (enc_len1) { free(enc_key1); @@ -2507,7 +2502,7 @@ static int lookup_one(struct autofs_point *ap, mapent = malloc(v_len + 1); if (!mapent) { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "malloc: %s", estr); ldap_value_free_len(bvValues); goto next; @@ -2527,7 +2522,7 @@ static int lookup_one(struct autofs_point *ap, mapent_len = new_size; } else { char *estr; - estr = strerror_r(errno, buf, MAX_ERR_BUF); + estr = strerror_r(errno, buf, sizeof(buf)); logerr(MODPREFIX "realloc: %s", estr); } }