[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
