From 4226691b79845d9b41041e2f64a3a241dcb99f4d Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 2 Jul 2017 10:30:48 +0100 Subject: [PATCH 1/1] Transform string_append_listele{,_n}() to proper expanding-string triplet interface (but do always maintain a nul-term string result). This avoids always copying the previous list version, and should do fewer allocs too. --- doc/doc-txt/ChangeLog | 3 + src/src/daemon.c | 8 ++- src/src/dkim.c | 6 +- src/src/expand.c | 22 +++--- src/src/functions.h | 4 +- src/src/readconf.c | 6 +- src/src/routers/rf_get_munge_headers.c | 6 +- src/src/string.c | 93 ++++++++++++-------------- src/src/tls.c | 3 +- src/src/tlscert-gnu.c | 14 ++-- src/src/tlscert-openssl.c | 21 +++--- src/src/utf8.c | 3 +- 12 files changed, 101 insertions(+), 88 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 8c6a1a5d3..7544e35a4 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -115,6 +115,9 @@ PP/06 Repair manualroute's ability to take options in any order, even if one HS/01 Cleanup, prevent repeated use of -p/-oMr (CVE-2017-1000369) +JH/17 Change the list-building routines interface to use the expanding-string + triplet model, for better allocation and copying behaviour. + Exim version 4.89 ----------------- diff --git a/src/src/daemon.c b/src/src/daemon.c index 06c2b258c..a05b79723 100644 --- a/src/src/daemon.c +++ b/src/src/daemon.c @@ -1173,6 +1173,8 @@ if (daemon_listen && !inetd_wait_mode) while ((s = string_nextinlist(&list, &sep, big_buffer, big_buffer_size))) if (!isdigit(*s)) { + int size = 0, len = 0; + list = tls_in.on_connect_ports; tls_in.on_connect_ports = NULL; sep = 0; @@ -1180,13 +1182,13 @@ if (daemon_listen && !inetd_wait_mode) { if (!isdigit(*s)) { - struct servent *smtp_service = getservbyname(CS s, "tcp"); + struct servent * smtp_service = getservbyname(CS s, "tcp"); if (!smtp_service) log_write(0, LOG_PANIC_DIE|LOG_CONFIG, "TCP port \"%s\" not found", s); - s= string_sprintf("%d", (int)ntohs(smtp_service->s_port)); + s = string_sprintf("%d", (int)ntohs(smtp_service->s_port)); } tls_in.on_connect_ports = string_append_listele(tls_in.on_connect_ports, - ':', s); + &size, &len, ':', s); } break; } diff --git a/src/src/dkim.c b/src/src/dkim.c index 03efb18d4..33e7bc84c 100644 --- a/src/src/dkim.c +++ b/src/src/dkim.c @@ -251,10 +251,12 @@ for (sig = dkim_signatures; sig; sig = sig->next) /* Build a colon-separated list of signing domains (and identities, if present) in dkim_signers */ if (sig->domain) - dkim_signers = string_append_listele(dkim_signers, ':', sig->domain); + dkim_signers = string_append_listele(dkim_signers, &dkim_signers_size, + &dkim_signers_ptr, ':', sig->domain); if (sig->identity) - dkim_signers = string_append_listele(dkim_signers, ':', sig->identity); + dkim_signers = string_append_listele(dkim_signers, &dkim_signers_size, + &dkim_signers_ptr, ':', sig->identity); /* Process next signature */ } diff --git a/src/src/expand.c b/src/src/expand.c index a68849dc3..a064e34e4 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -5985,7 +5985,9 @@ while (*s != 0) { uschar * dstitem; uschar * newlist = NULL; + int size = 0, len = 0; uschar * newkeylist = NULL; + int ksize = 0, klen = 0; uschar * srcfield; DEBUG(D_expand) debug_printf_indent("%s: $item = \"%s\"\n", name, srcitem); @@ -6030,33 +6032,33 @@ while (*s != 0) /* New-item sorts before this dst-item. Append new-item, then dst-item, then remainder of dst list. */ - newlist = string_append_listele(newlist, sep, srcitem); - newkeylist = string_append_listele(newkeylist, sep, srcfield); + newlist = string_append_listele(newlist, &size, &len, sep, srcitem); + newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, srcfield); srcitem = NULL; - newlist = string_append_listele(newlist, sep, dstitem); - newkeylist = string_append_listele(newkeylist, sep, dstfield); + newlist = string_append_listele(newlist, &size, &len, sep, dstitem); + newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, dstfield); while ((dstitem = string_nextinlist(&dstlist, &sep, NULL, 0))) { if (!(dstfield = string_nextinlist(&dstkeylist, &sep, NULL, 0))) goto sort_mismatch; - newlist = string_append_listele(newlist, sep, dstitem); - newkeylist = string_append_listele(newkeylist, sep, dstfield); + newlist = string_append_listele(newlist, &size, &len, sep, dstitem); + newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, dstfield); } break; } - newlist = string_append_listele(newlist, sep, dstitem); - newkeylist = string_append_listele(newkeylist, sep, dstfield); + newlist = string_append_listele(newlist, &size, &len, sep, dstitem); + newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, dstfield); } /* If we ran out of dstlist without consuming srcitem, append it */ if (srcitem) { - newlist = string_append_listele(newlist, sep, srcitem); - newkeylist = string_append_listele(newkeylist, sep, srcfield); + newlist = string_append_listele(newlist, &size, &len, sep, srcitem); + newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, srcfield); } dstlist = newlist; diff --git a/src/src/functions.h b/src/src/functions.h index 34f6434d0..41e3ec163 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -440,8 +440,8 @@ extern int stdin_feof(void); extern int stdin_ferror(void); extern int stdin_ungetc(int); extern uschar *string_append(uschar *, int *, int *, int, ...) WARN_UNUSED_RESULT; -extern uschar *string_append_listele(uschar *, uschar, const uschar *); -extern uschar *string_append_listele_n(uschar *, uschar, const uschar *, unsigned); +extern uschar *string_append_listele(uschar *, int *, int *, uschar, const uschar *) WARN_UNUSED_RESULT; +extern uschar *string_append_listele_n(uschar *, int *, int *, uschar, const uschar *, unsigned) WARN_UNUSED_RESULT; extern uschar *string_base62(unsigned long int); extern uschar *string_cat(uschar *, int *, int *, const uschar *) WARN_UNUSED_RESULT; extern uschar *string_catn(uschar *, int *, int *, const uschar *, int) WARN_UNUSED_RESULT; diff --git a/src/src/readconf.c b/src/src/readconf.c index fd9657e0e..eb44d1085 100644 --- a/src/src/readconf.c +++ b/src/src/readconf.c @@ -1899,9 +1899,13 @@ switch (type) const uschar * list = sptr; uschar * s; uschar * list_o = *str_target; + int size = 0, len = 0; + + if (list_o) + size = (len = Ustrlen(list_o)) + 1; while ((s = string_nextinlist(&list, &sep_i, NULL, 0))) - list_o = string_append_listele(list_o, sep_o, s); + list_o = string_append_listele(list_o, &size, &len, sep_o, s); if (list_o) *str_target = string_copy_malloc(list_o); } diff --git a/src/src/routers/rf_get_munge_headers.c b/src/src/routers/rf_get_munge_headers.c index ecb4ee097..745704f62 100644 --- a/src/src/routers/rf_get_munge_headers.c +++ b/src/src/routers/rf_get_munge_headers.c @@ -91,6 +91,10 @@ if (rblock->remove_headers) const uschar * list = rblock->remove_headers; int sep = ':'; uschar * s; + int size = 0, len = 0; + + if (*remove_headers) + size = (len = Ustrlen(*remove_headers)) + 1; while ((s = string_nextinlist(&list, &sep, NULL, 0))) if (!(s = expand_string(s))) @@ -104,7 +108,7 @@ if (rblock->remove_headers) } } else if (*s) - *remove_headers = string_append_listele(*remove_headers, ':', s); + *remove_headers = string_append_listele(*remove_headers, &size, &len, ':', s); } return OK; diff --git a/src/src/string.c b/src/src/string.c index 0d5a09703..53bcdfb7b 100644 --- a/src/src/string.c +++ b/src/src/string.c @@ -968,21 +968,43 @@ else *listptr = s; return buffer; } -#endif /* COMPILE_UTILITY */ -#ifndef COMPILE_UTILITY +static const uschar * +Ustrnchr(const uschar * s, int c, unsigned * len) +{ +unsigned siz = *len; +while (siz) + { + if (!*s) return NULL; + if (*s == c) + { + *len = siz; + return s; + } + s++; + siz--; + } +return NULL; +} + + /************************************************ * Add element to separated list * ************************************************/ -/* This function is used to build a list, returning -an allocated null-terminated growable string. The -given element has any embedded separator characters +/* This function is used to build a list, returning an allocated null-terminated +growable string. The given element has any embedded separator characters doubled. +Despite having the same growable-string interface as string_cat() the list is +always returned null-terminated. + Arguments: list points to the start of the list that is being built, or NULL if this is a new list that has no contents yet + sz (ptr to) amount of memory allocated for list; zero for a new list + off (ptr to) current list length in chars (insert point for next addition), + zero for a new list sep list separator character ele new element to be appended to the list @@ -990,78 +1012,49 @@ Returns: pointer to the start of the list, changed if copied for expansion. */ uschar * -string_append_listele(uschar * list, uschar sep, const uschar * ele) +string_append_listele(uschar * list, int * sz, int * off, + uschar sep, const uschar * ele) { -uschar * new = NULL; -int sz = 0, off = 0; uschar * sp; if (list) - { - new = string_cat (new, &sz, &off, list); - new = string_catn(new, &sz, &off, &sep, 1); - } + list = string_catn(list, sz, off, &sep, 1); while((sp = Ustrchr(ele, sep))) { - new = string_catn(new, &sz, &off, ele, sp-ele+1); - new = string_catn(new, &sz, &off, &sep, 1); + list = string_catn(list, sz, off, ele, sp-ele+1); + list = string_catn(list, sz, off, &sep, 1); ele = sp+1; } -new = string_cat(new, &sz, &off, ele); -new[off] = '\0'; -return new; +list = string_cat(list, sz, off, ele); +list[*off] = '\0'; +return list; } -static const uschar * -Ustrnchr(const uschar * s, int c, unsigned * len) -{ -unsigned siz = *len; -while (siz) - { - if (!*s) return NULL; - if (*s == c) - { - *len = siz; - return s; - } - s++; - siz--; - } -return NULL; -} - uschar * -string_append_listele_n(uschar * list, uschar sep, const uschar * ele, - unsigned len) +string_append_listele_n(uschar * list, int * sz, int * off, + uschar sep, const uschar * ele, unsigned len) { -uschar * new = NULL; -int sz = 0, off = 0; const uschar * sp; if (list) - { - new = string_cat (new, &sz, &off, list); - new = string_catn(new, &sz, &off, &sep, 1); - } + list = string_catn(list, sz, off, &sep, 1); while((sp = Ustrnchr(ele, sep, &len))) { - new = string_catn(new, &sz, &off, ele, sp-ele+1); - new = string_catn(new, &sz, &off, &sep, 1); + list = string_catn(list, sz, off, ele, sp-ele+1); + list = string_catn(list, sz, off, &sep, 1); ele = sp+1; len--; } -new = string_catn(new, &sz, &off, ele, len); -new[off] = '\0'; -return new; +list = string_catn(list, sz, off, ele, len); +list[*off] = '\0'; +return list; } -#endif /* COMPILE_UTILITY */ -#ifndef COMPILE_UTILITY /************************************************* * Add chars to string * *************************************************/ diff --git a/src/src/tls.c b/src/src/tls.c index a5cb35bd9..c93eb4579 100644 --- a/src/src/tls.c +++ b/src/src/tls.c @@ -264,6 +264,7 @@ uschar * ele; uschar * match = NULL; int len; uschar * list = NULL; +int size = 0, pos = 0; while ((ele = string_nextinlist(&mod, &insep, NULL, 0))) if (ele[0] != '>') @@ -278,7 +279,7 @@ while ((ele = string_nextinlist(CUSS &dn, &insep, NULL, 0))) if ( !match || Ustrncmp(ele, match, len) == 0 && ele[len] == '=' ) - list = string_append_listele(list, outsep, ele+len+1); + list = string_append_listele(list, &size, &pos, outsep, ele+len+1); return list; } diff --git a/src/src/tlscert-gnu.c b/src/src/tlscert-gnu.c index 296398ae9..3c6953a99 100644 --- a/src/src/tlscert-gnu.c +++ b/src/src/tlscert-gnu.c @@ -280,6 +280,7 @@ uschar * tls_cert_subject_altname(void * cert, uschar * mod) { uschar * list = NULL; +int lsize = 0, llen = 0; int index; size_t siz; int ret; @@ -332,7 +333,7 @@ for(index = 0;; index++) case GNUTLS_SAN_RFC822NAME: tag = US"MAIL"; break; default: continue; /* ignore unrecognised types */ } - list = string_append_listele(list, sep, + list = string_append_listele(list, &lsize, &llen, sep, match == -1 ? string_sprintf("%s=%s", tag, ele) : ele); } /*NOTREACHED*/ @@ -347,6 +348,7 @@ int ret; uschar sep = '\n'; int index; uschar * list = NULL; +int lsize = 0, llen = 0; if (mod) if (*mod == '>' && *++mod) sep = *mod++; @@ -361,8 +363,8 @@ for(index = 0;; index++) if (ret < 0) return g_err("gai", __FUNCTION__, ret); - list = string_append_listele(list, sep, - string_copyn(uri.data, uri.size)); + list = string_append_listele_n(list, &lsize, &llen, sep, + uri.data, uri.size); } /*NOTREACHED*/ @@ -384,6 +386,7 @@ size_t siz; uschar sep = '\n'; int index; uschar * list = NULL; +int lsize = 0, llen = 0; uschar * ele; if (mod) @@ -403,13 +406,12 @@ for(index = 0;; index++) return g_err("gc0", __FUNCTION__, ret); } - ele = store_get(siz+1); + ele = store_get(siz); if ((ret = gnutls_x509_crt_get_crl_dist_points( (gnutls_x509_crt_t)cert, index, ele, &siz, NULL, NULL)) < 0) return g_err("gc1", __FUNCTION__, ret); - ele[siz] = '\0'; - list = string_append_listele(list, sep, ele); + list = string_append_listele_n(list, &lsize, &llen, sep, ele, siz); } /*NOTREACHED*/ } diff --git a/src/src/tlscert-openssl.c b/src/src/tlscert-openssl.c index 690f95081..2e6ee8b3c 100644 --- a/src/src/tlscert-openssl.c +++ b/src/src/tlscert-openssl.c @@ -344,6 +344,7 @@ uschar * tls_cert_subject_altname(void * cert, uschar * mod) { uschar * list = NULL; +int lsize = 0, llen = 0; STACK_OF(GENERAL_NAME) * san = (STACK_OF(GENERAL_NAME) *) X509_get_ext_d2i((X509 *)cert, NID_subject_alt_name, NULL, NULL); uschar osep = '\n'; @@ -394,7 +395,7 @@ while (sk_GENERAL_NAME_num(san) > 0) ele = string_copyn(ele, len); if (Ustrlen(ele) == len) /* ignore any with embedded nul */ - list = string_append_listele(list, osep, + list = string_append_listele(list, &lsize, &llen, osep, match == -1 ? string_sprintf("%s=%s", tag, ele) : ele); } @@ -411,6 +412,7 @@ int adsnum = sk_ACCESS_DESCRIPTION_num(ads); int i; uschar sep = '\n'; uschar * list = NULL; +int size = 0, len = 0; if (mod) if (*mod == '>' && *++mod) sep = *mod++; @@ -420,11 +422,9 @@ for (i = 0; i < adsnum; i++) ACCESS_DESCRIPTION * ad = sk_ACCESS_DESCRIPTION_value(ads, i); if (ad && OBJ_obj2nid(ad->method) == NID_ad_OCSP) - { - uschar * ele = ASN1_STRING_data(ad->location->d.ia5); - int len = ASN1_STRING_length(ad->location->d.ia5); - list = string_append_listele_n(list, sep, ele, len); - } + list = string_append_listele_n(list, &size, &len, sep, + ASN1_STRING_data(ad->location->d.ia5), + ASN1_STRING_length(ad->location->d.ia5)); } sk_ACCESS_DESCRIPTION_free(ads); return list; @@ -441,6 +441,7 @@ int dpsnum = sk_DIST_POINT_num(dps); int i; uschar sep = '\n'; uschar * list = NULL; +int size = 0, len = 0; if (mod) if (*mod == '>' && *++mod) sep = *mod++; @@ -457,11 +458,9 @@ if (dps) for (i = 0; i < dpsnum; i++) if ( (np = sk_GENERAL_NAME_value(names, j)) && np->type == GEN_URI ) - { - uschar * ele = ASN1_STRING_data(np->d.uniformResourceIdentifier); - int len = ASN1_STRING_length(np->d.uniformResourceIdentifier); - list = string_append_listele_n(list, sep, ele, len); - } + list = string_append_listele_n(list, &size, &len, sep, + ASN1_STRING_data(np->d.uniformResourceIdentifier), + ASN1_STRING_length(np->d.uniformResourceIdentifier)); } sk_DIST_POINT_free(dps); return list; diff --git a/src/src/utf8.c b/src/src/utf8.c index 7b7b88f66..255383051 100644 --- a/src/src/utf8.c +++ b/src/src/utf8.c @@ -98,6 +98,7 @@ string_domain_alabel_to_utf8(const uschar * alabel, uschar ** err) const uschar * label; int sep = '.'; uschar * s = NULL; +int size = 0, len = 0; while (label = string_nextinlist(&alabel, &sep, NULL, 0)) if ( string_is_alabel(label) @@ -105,7 +106,7 @@ while (label = string_nextinlist(&alabel, &sep, NULL, 0)) ) return NULL; else - s = string_append_listele(s, '.', label); + s = string_append_listele(s, &size, &len, '.', label); return s; #else -- 2.30.2