[2024-01-31 23:44] Omar Polo <o...@omarpolo.com> > On 2024/01/27 18:33:35 +0100, Philipp <phil...@bureaucracy.de> wrote: > > --- a/extras/tables/table-ldap/aldap.c > > +++ b/extras/tables/table-ldap/aldap.c > > [...] > > @@ -1243,12 +1243,41 @@ parseval(char *p, size_t len) > > (void)strlcpy(hex, cp + j + 1, sizeof(hex)); > > buffer[i] = (char)strtoumax(hex, NULL, 16); > > j += 3; > > + } else if (cp[j] == '%') { > > + switch (cp[j + 1]) { > > + case '%': > > + buffer[i] = '%'; > > + j += 2; > > + break; > > + case 's': > > + if (!key) { > > + free(buffer); > > + return NULL; > > + } > > + keylen = strlen(key); > > + newsize = size + keylen; > > + if ((newbuffer = realloc(buffer, newsize)) == > > NULL) { > > + free(buffer); > > + return NULL; > > + } > > + buffer = newbuffer; > > + size = newsize; > > + memcpy(buffer + i, key, keylen); > > + i += keylen - 1; > > Here we can underflow when keylen is zero. Probably it's not such a big > deal since all these vars are unsigned and we're going to increment x > again at the next loop iteration (which is always executed), rewrapping > again, but it's not nice :/ > > (also, I'm not sure we can reach the case of a key with length zero, but > since we care to NULL check, we should handle "" as a key too IMHO.)
Thanks, fixed version is attached. Philipp
From 39bc4faaddc71415e0e4ccdf2af5cd946e2030ab Mon Sep 17 00:00:00 2001 From: Philipp Takacs <phil...@bureaucracy.de> Date: Tue, 23 Jan 2024 22:06:24 +0100 Subject: [PATCH 1/5] table-ldap do the string replace in the ldap parser This allows filter which uses the key multible times. I.e: (|(mail=%s)(username=%s)) --- extras/tables/table-ldap/aldap.c | 63 ++++++++++++++++++++------- extras/tables/table-ldap/aldap.h | 2 +- extras/tables/table-ldap/table_ldap.c | 11 +++-- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/extras/tables/table-ldap/aldap.c b/extras/tables/table-ldap/aldap.c index c38aae5..dff6432 100644 --- a/extras/tables/table-ldap/aldap.c +++ b/extras/tables/table-ldap/aldap.c @@ -32,12 +32,12 @@ #define ALDAP_VERSION 3 static struct ber_element *ldap_parse_search_filter(struct ber_element *, - char *); + char *, const char *); static struct ber_element *ldap_do_parse_search_filter( - struct ber_element *, char **); + struct ber_element *, char **, const char *); char **aldap_get_stringset(struct ber_element *); char *utoa(char *); -char *parseval(char *, size_t); +char *parseval(char *, size_t, const char *); int aldap_create_page_control(struct ber_element *, int, struct aldap_page_control *); @@ -156,7 +156,7 @@ fail: int aldap_search(struct aldap *ldap, char *basedn, enum scope scope, char *filter, - char **attrs, int typesonly, int sizelimit, int timelimit, + const char *key, char **attrs, int typesonly, int sizelimit, int timelimit, struct aldap_page_control *page) { struct ber_element *root = NULL, *ber, *c; @@ -181,7 +181,7 @@ aldap_search(struct aldap *ldap, char *basedn, enum scope scope, char *filter, goto fail; } - if ((ber = ldap_parse_search_filter(ber, filter)) == NULL) { + if ((ber = ldap_parse_search_filter(ber, filter, key)) == NULL) { ldap->err = ALDAP_ERR_PARSER_ERROR; goto fail; } @@ -753,7 +753,7 @@ aldap_get_stringset(struct ber_element *elm) * NULL, parse failed */ static struct ber_element * -ldap_parse_search_filter(struct ber_element *ber, char *filter) +ldap_parse_search_filter(struct ber_element *ber, char *filter, const char *key) { struct ber_element *elm; char *cp; @@ -765,7 +765,7 @@ ldap_parse_search_filter(struct ber_element *ber, char *filter) return (NULL); } - if ((elm = ldap_do_parse_search_filter(ber, &cp)) == NULL) + if ((elm = ldap_do_parse_search_filter(ber, &cp, key)) == NULL) return (NULL); if (*cp != '\0') { @@ -795,7 +795,7 @@ ldap_parse_search_filter(struct ber_element *ber, char *filter) * */ static struct ber_element * -ldap_do_parse_search_filter(struct ber_element *prev, char **cpp) +ldap_do_parse_search_filter(struct ber_element *prev, char **cpp, const char *key) { struct ber_element *elm, *root = NULL; char *attr_desc, *attr_val, *parsed_val, *cp; @@ -827,7 +827,7 @@ ldap_do_parse_search_filter(struct ber_element *prev, char **cpp) while (*cp == '(') { if ((elm = - ldap_do_parse_search_filter(elm, &cp)) == NULL) + ldap_do_parse_search_filter(elm, &cp, key)) == NULL) goto bad; } @@ -841,7 +841,7 @@ ldap_do_parse_search_filter(struct ber_element *prev, char **cpp) ber_set_header(root, BER_CLASS_CONTEXT, LDAP_FILT_NOT); cp++; /* now points to sub-filter */ - if (ldap_do_parse_search_filter(root, &cp) == NULL) + if (ldap_do_parse_search_filter(root, &cp, key) == NULL) goto bad; if (*cp != ')') /* trailing `)` of filter */ @@ -932,7 +932,7 @@ ldap_do_parse_search_filter(struct ber_element *prev, char **cpp) else type = LDAP_FILT_SUBS_ANY; - if ((parsed_val = parseval(attr_val, len)) == + if ((parsed_val = parseval(attr_val, len, key)) == NULL) goto callfail; elm = ber_add_nstring(elm, parsed_val, @@ -947,7 +947,7 @@ ldap_do_parse_search_filter(struct ber_element *prev, char **cpp) break; } - if ((parsed_val = parseval(attr_val, len)) == NULL) + if ((parsed_val = parseval(attr_val, len, key)) == NULL) goto callfail; elm = ber_add_nstring(elm, parsed_val, strlen(parsed_val)); free(parsed_val); @@ -1235,13 +1235,13 @@ utoa(char *u) * the argument u should be a NULL terminated sequence of ASCII bytes. */ char * -parseval(char *p, size_t len) +parseval(char *p, size_t len, const char *key) { char hex[3]; char *cp = p, *buffer, *newbuffer; - size_t size, newsize, i, j; + size_t size, newsize, i, j, keylen; - size = 50; + size = len + 1; if ((buffer = calloc(1, size)) == NULL) return NULL; @@ -1260,12 +1260,45 @@ parseval(char *p, size_t len) (void)strlcpy(hex, cp + j + 1, sizeof(hex)); buffer[i] = (char)strtoumax(hex, NULL, 16); j += 3; + } else if (cp[j] == '%') { + switch (cp[j + 1]) { + case '%': + buffer[i] = '%'; + j += 2; + break; + case 's': + if (!key) { + free(buffer); + return NULL; + } + keylen = strlen(key); + if (!keylen) { + j += 2; + break; + } + newsize = size + keylen; + if ((newbuffer = realloc(buffer, newsize)) == NULL) { + free(buffer); + return NULL; + } + buffer = newbuffer; + size = newsize; + memcpy(buffer + i, key, keylen); + i += keylen - 1; + j += 2; + break; + default: + buffer[i] = '%'; + j++; + break; + } } else { buffer[i] = cp[j]; j++; } } + buffer[i] = 0; return buffer; } diff --git a/extras/tables/table-ldap/aldap.h b/extras/tables/table-ldap/aldap.h index 6f80e2c..60159b4 100644 --- a/extras/tables/table-ldap/aldap.h +++ b/extras/tables/table-ldap/aldap.h @@ -195,7 +195,7 @@ void aldap_freemsg(struct aldap_message *); int aldap_bind(struct aldap *, char *, char *); int aldap_unbind(struct aldap *); -int aldap_search(struct aldap *, char *, enum scope, char *, char **, int, int, int, struct aldap_page_control *); +int aldap_search(struct aldap *, char *, enum scope, char *, const char *, char **, int, int, int, struct aldap_page_control *); int aldap_get_errno(struct aldap *, const char **); int aldap_get_resultcode(struct aldap_message *); diff --git a/extras/tables/table-ldap/table_ldap.c b/extras/tables/table-ldap/table_ldap.c index 3d3b0f1..2b7d2d3 100644 --- a/extras/tables/table-ldap/table_ldap.c +++ b/extras/tables/table-ldap/table_ldap.c @@ -397,7 +397,7 @@ table_ldap_lookup(int service, struct dict *params, const char *key, char *dst, } static int -ldap_query(const char *filter, char **attributes, char ***outp, size_t n) +ldap_query(const char *filter, const char *key, char **attributes, char ***outp, size_t n) { struct aldap_message *m = NULL; struct aldap_page_control *pg = NULL; @@ -405,15 +405,18 @@ ldap_query(const char *filter, char **attributes, char ***outp, size_t n) size_t i; char basedn__[MAX_LDAP_BASELEN]; char filter__[MAX_LDAP_FILTERLEN]; + char key__[MAX_LDAP_IDENTIFIER]; if (strlcpy(basedn__, basedn, sizeof basedn__) >= sizeof basedn__) return -1; if (strlcpy(filter__, filter, sizeof filter__) >= sizeof filter__) return -1; + if (strlcpy(key__, key, sizeof key__) >= sizeof key__) + return -1; found = 0; do { if ((ret = aldap_search(aldap, basedn__, LDAP_SCOPE_SUBTREE, - filter__, NULL, 0, 0, 0, pg)) == -1) { + filter__, key__, NULL, 0, 0, 0, pg)) == -1) { log_debug("ret=%d", ret); return -1; } @@ -478,14 +481,14 @@ ldap_run_query(int type, const char *key, char *dst, size_t sz) return -1; } - if (snprintf(filter, sizeof(filter), q->filter, key) + if (snprintf(filter, sizeof(filter), "%s", q->filter) >= (int)sizeof(filter)) { log_warnx("warn: filter too large"); return -1; } memset(res, 0, sizeof(res)); - ret = ldap_query(filter, q->attrs, res, q->attrn); + ret = ldap_query(filter, key, q->attrs, res, q->attrn); if (ret <= 0 || dst == NULL) goto end; -- 2.39.2