On 2023/03/18 13:10:49 -0600, "Todd C. Miller" <[email protected]> wrote:
> Thanks, I was unable to get a backtrace so this really helped. I
> think the safest thing to do is to just return an error if the
> expanded string is NULL. I'm not sure if there are other expansions
> that can also be NULL here.
>
> Alternately, we could move the check to be specific to the
> else if (!strcasecmp("mda", rtoken)) {
> ...
> }
>
> block.
>
> - todd
>
> Index: mda_variables.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/mda_variables.c,v
> retrieving revision 1.7
> diff -u -p -u -r1.7 mda_variables.c
> --- mda_variables.c 14 Jun 2021 17:58:15 -0000 1.7
> +++ mda_variables.c 18 Mar 2023 19:03:11 -0000
> @@ -51,7 +51,7 @@ mda_expand_token(char *dest, size_t len,
> {
> char rtoken[MAXTOKENLEN];
> char tmp[EXPAND_BUFFER];
> - const char *string;
> + const char *string = NULL;
> char *lbracket, *rbracket, *content, *sep, *mods;
> ssize_t i;
> ssize_t begoff, endoff;
> @@ -159,6 +159,8 @@ mda_expand_token(char *dest, size_t len,
> return -1;
>
> if (string != tmp) {
> + if (string == NULL)
> + return -1;
> if (strlcpy(tmp, string, sizeof tmp) >= sizeof tmp)
> return -1;
> string = tmp;
If cscope is not missing anything, mda_expand_token is only called by
mda_expand_format which is only called by mda_unpriv.
Now, mda_unpriv() always pass a NULL mda_command the first time (the
last argument)
mda_unpriv.c:
46 if (mda_expand_format(mda_exec, sizeof mda_exec, deliver,
47 &deliver->userinfo, NULL) == -1)
48 errx(1, "mda command line could not be expanded");
So (assuming I'm reading everything right) isn't your diff completely
nuking the "%{mda}" handling? I'm not sure how it could/should be
used, but with my test .forward of "|%{mda}" I always get
Mar 18 21:02:04 venera smtpd[86585]: 50d046b923383a51 mda delivery
evpid=c1bac2354adc45df from=<joe@localhost> to=<op@localhost>
rcpt=<op@localhost> user=op delay=1h52m40s result=PermFail stat=Error ("smtpd:
mda command line could not be expanded")
I was tempted to say that if string is NULL we should consider it as
it were the empty string "", but I'm not sure. I'm not following what
mda_unpriv does. It should probably pass `mda_command' instead of
NULL.
