On 2024/01/27 18:33:35 +0100, Philipp <phil...@bureaucracy.de> wrote:
> A I forgott the patch.
>
> [2024-01-27 12:11] Philipp <phil...@bureaucracy.de>
> > I have noticed that the table-ldap uses printf() to replace the '%s' of
> > the filter with the search key. This has some problems. The biggest one
> > is you can use the key only once in the filter. So a filter like:
> >
> > > (|(mail=%s)(uid=%s))
> >
> > doesn't work.
> >
> > To fix this I have moved the replacement to the parsval function of aldap.
> >
> > Philipp
Thanks for this. Reads fine to me, one nit below
> --- 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.)
