On Sun, Jun 30, 2019 at 04:54:28PM +0200, Martijn van Duren wrote:
> Found by Mischa Peters by running some experimental code of mine.
>
> When chaining two proc-based filters together the second proc will not
> get its parameter correctly, since lka_filter_process_response won't get
> a parameter from the proceed keyword, resulting in a "...|(null)"
>
> The solution seems to be to save the parameter in the filter_session
> struct.
>
> While here remove some useless if(NULL) checks and fix two memory leaks
> (fs->helo and fs->mail_from).
>
> Mischa has been running with this diff for several days now without
> issue.
>
> OK?
>
yes ok gilles@
> Index: lka_filter.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 lka_filter.c
> --- lka_filter.c 2 May 2019 11:39:45 -0000 1.36
> +++ lka_filter.c 30 Jun 2019 14:50:58 -0000
> @@ -41,7 +41,7 @@ struct filter;
> struct filter_session;
> static void filter_protocol_internal(struct filter_session *, uint64_t *,
> uint64_t, enum filter_phase, const char *);
> static void filter_protocol(uint64_t, enum filter_phase, const char *);
> -static void filter_protocol_next(uint64_t, uint64_t, enum filter_phase,
> const char *);
> +static void filter_protocol_next(uint64_t, uint64_t, enum filter_phase);
> static void filter_protocol_query(struct filter *, uint64_t, uint64_t,
> const char *, const char *);
>
> static void filter_data_internal(struct filter_session *, uint64_t,
> uint64_t, const char *);
> @@ -68,6 +68,8 @@ struct filter_session {
> uint64_t id;
> struct io *io;
>
> + char *lastparam;
> +
> char *filter_name;
> struct sockaddr_storage ss_src;
> struct sockaddr_storage ss_dest;
> @@ -337,6 +339,9 @@ lka_filter_end(uint64_t reqid)
>
> fs = tree_xpop(&sessions, reqid);
> free(fs->rdns);
> + free(fs->helo);
> + free(fs->mail_from);
> + free(fs->lastparam);
> free(fs);
> log_trace(TRACE_FILTERS, "%016"PRIx64" filters session-end", reqid);
> }
> @@ -504,7 +509,7 @@ lka_filter_process_response(const char *
> return 1;
> }
>
> - filter_protocol_next(token, reqid, 0, parameter);
> + filter_protocol_next(token, reqid, 0);
> return 1;
> }
>
> @@ -658,14 +663,11 @@ filter_protocol(uint64_t reqid, enum fil
> switch (phase) {
> case FILTER_HELO:
> case FILTER_EHLO:
> - if (fs->helo)
> - free(fs->helo);
> + free(fs->helo);
> fs->helo = xstrdup(param);
> break;
> case FILTER_MAIL_FROM:
> - if (fs->mail_from)
> - free(fs->mail_from);
> -
> + free(fs->mail_from);
> fs->mail_from = xstrdup(param + 1);
> *strchr(fs->mail_from, '>') = '\0';
> param = fs->mail_from;
> @@ -683,13 +685,17 @@ filter_protocol(uint64_t reqid, enum fil
> default:
> break;
> }
> +
> + free(fs->lastparam);
> + fs->lastparam = xstrdup(param);
> +
> filter_protocol_internal(fs, &token, reqid, phase, param);
> if (nparam)
> free(nparam);
> }
>
> static void
> -filter_protocol_next(uint64_t token, uint64_t reqid, enum filter_phase
> phase, const char *param)
> +filter_protocol_next(uint64_t token, uint64_t reqid, enum filter_phase phase)
> {
> struct filter_session *fs;
>
> @@ -697,7 +703,7 @@ filter_protocol_next(uint64_t token, uin
> if ((fs = tree_get(&sessions, reqid)) == NULL)
> return;
>
> - filter_protocol_internal(fs, &token, reqid, phase, param);
> + filter_protocol_internal(fs, &token, reqid, phase, fs->lastparam);
> }
>
> static void
--
Gilles Chehade @poolpOrg
https://www.poolp.org patreon: https://www.patreon.com/gilles
