On 2023/03/18 15:58:14 -0600, Todd C. Miller <mill...@openbsd.org> wrote:
> On Sat, 18 Mar 2023 21:17:36 +0100, Omar Polo wrote:
> 
> > 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)
> 
> In smtpd.conf, under FORMAT SPECIFIERS:
> 
>        %{mda}               mda command, only available for mda wrappers
> 
> It is only the second call that is for the mda wrapper so I think
> it is fine for %{mda} to be unavailable here, as documented.  However,
> perhaps it would be better to pass in "" instead of NULL for the
> first call.

Sorry, I misunderstood the meaning of the mda wrapper and so also what
your patch was effectively doing.

I've verified that it works as intended; it expands %{mda} inside a
`mda wrapper' directive but rejects it otherwise at runtime (for both
instance in the configuration or ~/.forward files.)

> > 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");
> 
> It doesn't really make sense to expand %{mda} in this context since
> it is the result of mda_expand_format() that is used to generate
> mda_command.  So I think the question is whether or not this should
> result in an error parsing the .forward file or just silently fail
> by expanding to an empty string.

I think that it's better to treat it as an error instead of silently
returning an empty string.  The downside is that some rules could fail
at run time when you attempt to use %{mda} outside a `mda wrapper'
directive, but it doesn't make sense to use %{mda} in other contexts
anyway.

I think your patch is correct; FWIW it's OK op@

Thanks!  (and sorry for the misunderstanding :-)

Reply via email to